Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPL-777 Refactor the crawler migrations [c=m-l, v=2*] #173

Open
2 tasks
Chris-Friend opened this issue Dec 11, 2020 · 0 comments
Open
2 tasks

GPL-777 Refactor the crawler migrations [c=m-l, v=2*] #173

Chris-Friend opened this issue Dec 11, 2020 · 0 comments

Comments

@Chris-Friend
Copy link
Contributor

Chris-Friend commented Dec 11, 2020

User story
As developers we want to easily be able to understand, update and test different pieces of functionality. This is made more difficult if a single file is given multiple purposes. We should aim to separate code with different purposes into separate files. This way we can use mocking to reduce test complexity, but it will also allow us to group together common functionality in shared helper files.

For the case of these migrations, we have the following pieces of functionality:

  • script-type logic that forms the migration
  • the methods called at each step in the script-type logic

We should aim to separate code that satisfies the different purposes into separate files.

We also want to avoid duplication of code. The migrations, especially the new ones, implement common steps that should be extracted out to shared locations.

Who are the primary contacts for this story
Chris F

Steps to Aid Splitting the Update Dart Migration

  • Have migrations log instead of print
  • Move the implementation of, e.g. for the update_dart migration, migrate_all_dbs in dart_samples_update_helper.py to the run method of update_dart.py. Keep all other methods in the helper file
  • Create corresponding test file, moving any relevant script-type tests from helper
  • Add more tests to the new script-type test file, including (mocked) end-to-end tests
  • Add more tests for the helper methods left in the original file
  • Do some integration testing: either in the development environment; or by adding some tests that have less mocking (e.g. do not mock the databases, but note that this will be easier to break than traditional unit tests). Specifically we want to ensure the complex steps, e.g. the get cherrypicked samples query, work as expected

Acceptance criteria
To be considered successful the solution must allow:

  • Separation of script-type logic in the update_dart migration from the helper methods called as part of the script, and sufficient testing
  • Consolidation of common/shared methods into single, tested instances

Dependencies
This story is blocked by the following dependencies:

Additional context
As the update_dart migration wasn't/isn't "fully" tested, some issues were found when updating it (above linked story), and more still may remain

@Chris-Friend Chris-Friend added Enhancement New feature or request Beckman integration Beckman integration labels Dec 11, 2020
@Chris-Friend Chris-Friend self-assigned this Dec 11, 2020
@Chris-Friend Chris-Friend removed the Beckman integration Beckman integration label Dec 14, 2020
@Chris-Friend Chris-Friend changed the title GPL-nnn Refactor the update_dart migration GPL-nnn Refactor the crawler migrations Dec 14, 2020
@rl15 rl15 changed the title GPL-nnn Refactor the crawler migrations GPL-777 Refactor the crawler migrations Dec 14, 2020
@rl15 rl15 added Deployment process improvement and removed Enhancement New feature or request labels Jan 7, 2021
@Chris-Friend Chris-Friend removed their assignment Jan 14, 2021
@rl15 rl15 changed the title GPL-777 Refactor the crawler migrations GPL-777 Refactor the crawler migrations [c=m-l, v=2*] Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants