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

Additional information and better logic for the acceptance test feature #1085

Open
isabelle-dr opened this issue Jan 7, 2022 · 0 comments
Open
Labels
epic Used for bigger pieces of work. Usually split between several issues. status: Ready An issue that is ready to be worked on. tests Anything related to our tests.
Milestone

Comments

@isabelle-dr
Copy link
Contributor

isabelle-dr commented Jan 7, 2022

The "acceptance test" has been created to inform the developer of the impact a PR on the datasets available on the Mobility Database, in order to:

  1. for the developer: make sure the code logic is working properly (if the code added is a rule with a WARNING severity and the acceptance test is flagged, there is a problem with the code)
  2. for the community: make an informed decision on what to do if the new rule is truly resulting in previously valid datasets becoming invalid. This will depend on the results of the acceptance test and it could be: (i) adding a grace period for producers to fix their feed, or (ii) changing the specification to reflect practice.l

The first version of this feature (built in PR #848, included in the 3.0.0 release) allows our users to:

  • at each commit, see a GH action if the code written results in more than 1% of datasets containing a new error occurrence.
  • in the GH action comment, see:
    • how many datasets did the test run on in total
    • how many datasets are corrupted
    • a downloadable acceptance_test_report, containing the list of dataset ID's that contain an additional error, the error type responsible, and the number of occurrences of that error in each dataset
    • a downloadable corrupted_sources_report containing the list of corrupted dataset ID's (meaning we don't know how the PR impacts them)
  • a specific commit message [acceptance test skip] can be used to prohibit this test from running (if fixing a typo, working on documentation)

Outcome

We evaluated that in order to provide developers with all the information they need, the following changes need to be added to this feature. This list will be updated as we gather more feedback on this feature.

  • as a developer writing a new rule, I want to know if my PR results in one additional occurrence of the error type
  • as a developer modifying an existing rule, I want to know
    • how many datasets were already containing the notice emitted by this rule
    • how many NEW datasets would contain the notice if my PR gets merged
    • how many datasets would NOT contain the notice anymore if my PR gets merged
    • as a result, how many datasets would contain the notice in total if my PR gets merged
    • which datasets are going from valid to invalid if this PR gets merges?
    • which datasets are going from invalid to valid if this PR gets merged?

How will this work?

This additional information could be shared by providing a Google Colab notebook with code snippets to run to get these analytics, or by changing the acceptance test report.

Note

The architecture of the Mobility Database is currently under reconsideration, and since this feature depends on it, we will wait until a decision has been made (February 2022) before moving forward with these changes.

@isabelle-dr isabelle-dr added the epic Used for bigger pieces of work. Usually split between several issues. label Jan 7, 2022
@isabelle-dr isabelle-dr added this to the Future work milestone Jan 7, 2022
@isabelle-dr isabelle-dr mentioned this issue Jan 7, 2022
4 tasks
@isabelle-dr isabelle-dr modified the milestones: Future work, v3.2.0, v3.2.0 - improve acceptance tests, v3.3.0 - improvements for the acceptance test Jan 9, 2022
@isabelle-dr isabelle-dr added the tests Anything related to our tests. label Oct 3, 2022
@isabelle-dr isabelle-dr removed this from the Acceptance test improvements milestone Oct 3, 2022
@isabelle-dr isabelle-dr added the status: Ready An issue that is ready to be worked on. label Jun 19, 2023
@isabelle-dr isabelle-dr modified the milestones: Next, Future work Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Used for bigger pieces of work. Usually split between several issues. status: Ready An issue that is ready to be worked on. tests Anything related to our tests.
Projects
None yet
Development

No branches or pull requests

1 participant