Closed Bug 1672691 Opened 4 years ago Closed 4 years ago

Add integration tests to beetmoverscript

Categories

(Release Engineering :: Release Automation, enhancement)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jlorenzo, Assigned: alex.lopez.zorzano, Mentored)

References

Details

Attachments

(4 files)

Context

Alex Lopez did an incredible job in bug 1600916! We hit a few issues along the way, but Alex kept up and was willing to provide fixes any time.

Most of the issues we faced could have been avoided if we had a few integration tests:

  • We wouldn't have to revert the changes on the worker, if we tested our changes against a geckoview task payload. That actually lead us to update the docstring[1] so people who change this function may have a way to remember.
  • We wouldn't have to wait for a try push to see that a renamed function wasn't updated everywhere it's called.

I don't blame people for not testing this. It's hard to remember everything. I'm fairly one of us won't see the docstring and will forget to test everything out. Therefore, test cases are a way to prevent that. Aki suggested we add integration tests with task payloads for different projects (android-components, application-services, glean, geckoview, Firefox, etc.). I agree this is the right level of abstraction.

Proposed solution

Good news is: beetmover is quite similar to other scriptworker-scripts we have and some of them do have integration tests. The recent githubscript is an example[2]. Let's first create a similar test for beetmoverscript. Then, in a subsequent PR expand to cover the test cases mentioned above.

To be more explicit, in the context of scriptworker-scripts, an integration test means: we test beetmoverscript as a unique entity. We make sure that, given an input (here a task payload), we interact with AWS S3 correctly. So we will:

  1. set up the task payload, and maybe the worker config (which is in a json file)
  2. mock the boto3 library https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html
  3. run the main function of beetmoverscript
  4. assert that the boto3 library was called the way we expected

Alex is motivated to add them. I'm happy to mentor him.

[1] https://github.com/mozilla-releng/scriptworker-scripts/pull/279
[2] https://github.com/mozilla-releng/scriptworker-scripts/blob/fc50116c91458baa33e4700d2010bbb28f96b9b6/githubscript/tests/test_integration.py

Attachment #9184648 - Attachment description: Bug 1672691 - part 2: [beetmover] Add integration tests to beetmover (more cases) → Bug 1672691 - part 2: [beetmoverscript] Add integration tests to beetmover (more cases)

Excellent work, Alex! Your tests enable our team to catch up with some years-old technical debt! Thank you so much!

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Glad to be of help!

Component: Release Automation: Uploading → Release Automation
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: