feat: Fail acceptance tests on significant change in warnings #1377

bdferris-v2 · 2023-04-04T21:44:27Z

Summary:

As discussed in #1349, we'd like to fail acceptance tests if the number of added or dropped warnings is above our target thresholds, similar to existing behavior for errors.

While the actual code change is minor (two lines to the existing failure clause), I took this opportunity to move a bunch of code out outputcomparator/cli/Main into a dedicated comparator class for easier unit testing. I add more unit tests for failure triggering to go along with the change.

Please make sure these boxes are checked before submitting your pull request - thanks!

Run the unit tests with gradle test to make sure you didn't break anything
Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
Linked all relevant issues

…idationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds.

github-actions · 2023-04-04T23:24:06Z

✅ Rule acceptance tests passed.
New Errors: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1424 sources (~0 %) are corrupted.
Commit: cf25cdc
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

davidgamez

LGTM, nice code clean-up!

davidgamez · 2023-04-05T01:11:24Z

...java/org/mobilitydata/gtfsvalidator/outputcomparator/cli/ValidationReportComparatorTest.java

+            "❌ Invalid acceptance test.\n"
+                + "New Errors: 1 out of 2 datasets (~50%) are invalid due to code change, which is above the provided threshold of 25%.\n"
+                + "Dropped Errors: 0 out of 2 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 25%.\n"
+                + "New Warnings: 0 out of 2 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 25%.\n"
+                + "Dropped Warnings: 0 out of 2 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 25%.\n"
+                + "0 out of 2 sources (~0 %) are corrupted.\n"
+                + "❌ Invalid acceptance test.\n");


Nice error output 🚀

github-actions · 2023-04-05T17:23:26Z

✅ Rule acceptance tests passed.
New Errors: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1424 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1424 sources (~0 %) are corrupted.
Commit: 124de3f
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

…tyData#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator.

isabelle-dr · 2023-04-12T23:41:23Z

This is awesome 😍

* chore: fix deprecated dependency * chore: auto-format * feat: handle missing RULES.md * feat: refactor upload process and handle more errors * Add a more descriptive test failure message to NoticeFieldsTest. (#1369) * Docs: Update notice migration file (#1357) * refactor migration file * update documentation with new file * fix wording * Make easier to read * remove colors --------- Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> * feat: Fail acceptance tests on significant change in warnings (#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator. --------- Co-authored-by: Ryon Coleman <ryon55@gmail.com> Co-authored-by: Brian Ferris <bdferris+v2@google.com> Co-authored-by: isabelle-dr <isabelle@mobilitydata.org> Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> Co-authored-by: David Gamez <1192523+davidgamez@users.noreply.github.com>

…1384) * chore: fix deprecated dependency * chore: auto-format * feat: handle missing RULES.md * feat: refactor upload process and handle more errors * Add a more descriptive test failure message to NoticeFieldsTest. (#1369) * Docs: Update notice migration file (#1357) * refactor migration file * update documentation with new file * fix wording * Make easier to read * remove colors --------- Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> * feat: Fail acceptance tests on significant change in warnings (#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator. * Add sentry script to static html header * Add sentry script to web service * Remove sentry properties from source control --------- Co-authored-by: Ryon Coleman <ryon55@gmail.com> Co-authored-by: Brian Ferris <bdferris+v2@google.com> Co-authored-by: isabelle-dr <isabelle@mobilitydata.org> Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com>

…tyData#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator.

* chore: fix deprecated dependency * chore: auto-format * feat: handle missing RULES.md * feat: refactor upload process and handle more errors * Add a more descriptive test failure message to NoticeFieldsTest. (MobilityData#1369) * Docs: Update notice migration file (MobilityData#1357) * refactor migration file * update documentation with new file * fix wording * Make easier to read * remove colors --------- Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> * feat: Fail acceptance tests on significant change in warnings (MobilityData#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator. --------- Co-authored-by: Ryon Coleman <ryon55@gmail.com> Co-authored-by: Brian Ferris <bdferris+v2@google.com> Co-authored-by: isabelle-dr <isabelle@mobilitydata.org> Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> Co-authored-by: David Gamez <1192523+davidgamez@users.noreply.github.com>

…obilityData#1384) * chore: fix deprecated dependency * chore: auto-format * feat: handle missing RULES.md * feat: refactor upload process and handle more errors * Add a more descriptive test failure message to NoticeFieldsTest. (MobilityData#1369) * Docs: Update notice migration file (MobilityData#1357) * refactor migration file * update documentation with new file * fix wording * Make easier to read * remove colors --------- Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> * feat: Fail acceptance tests on significant change in warnings (MobilityData#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator. * Add sentry script to static html header * Add sentry script to web service * Remove sentry properties from source control --------- Co-authored-by: Ryon Coleman <ryon55@gmail.com> Co-authored-by: Brian Ferris <bdferris+v2@google.com> Co-authored-by: isabelle-dr <isabelle@mobilitydata.org> Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com>

* chore: fix deprecated dependency * chore: auto-format * feat: handle missing RULES.md * feat: refactor upload process and handle more errors * Add a more descriptive test failure message to NoticeFieldsTest. (#1369) * Docs: Update notice migration file (#1357) * refactor migration file * update documentation with new file * fix wording * Make easier to read * remove colors --------- Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com> * feat: Fail acceptance tests on significant change in warnings (#1377) * Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds. * Additional unit tests for ValidationReportComparator. * Add sentry script to static html header * Add sentry script to web service * Remove sentry properties from source control * feat: add newrelic agent to web service --------- Co-authored-by: Ryon Coleman <ryon55@gmail.com> Co-authored-by: Brian Ferris <bdferris+v2@google.com> Co-authored-by: isabelle-dr <isabelle@mobilitydata.org> Co-authored-by: Emma Blue <90632321+emmambd@users.noreply.github.com>

bdferris-v2 added 2 commits April 4, 2023 14:08

Refactor logic out of of outputcomparator/cli/Main and into a new Val…

6cb2141

…idationReportComparator class for easier testing. In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds.

Additional unit tests for ValidationReportComparator.

90e2f86

bdferris-v2 linked an issue Apr 4, 2023 that may be closed by this pull request

Acceptance Tests only report changes in ERROR notices, not WARNINGS or INFOs #1349

Closed

bdferris-v2 requested a review from davidgamez April 4, 2023 21:44

davidgamez approved these changes Apr 5, 2023

View reviewed changes

Merge branch 'master' into issue/1349/accpetance_test_warnings

353470f

bdferris-v2 merged commit 0213fbd into master Apr 5, 2023

bdferris-v2 deleted the issue/1349/accpetance_test_warnings branch April 5, 2023 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Fail acceptance tests on significant change in warnings #1377

feat: Fail acceptance tests on significant change in warnings #1377

bdferris-v2 commented Apr 4, 2023

github-actions bot commented Apr 4, 2023

davidgamez left a comment

davidgamez Apr 5, 2023

github-actions bot commented Apr 5, 2023

isabelle-dr commented Apr 12, 2023

feat: Fail acceptance tests on significant change in warnings #1377

feat: Fail acceptance tests on significant change in warnings #1377

Conversation

bdferris-v2 commented Apr 4, 2023

github-actions bot commented Apr 4, 2023

davidgamez left a comment

Choose a reason for hiding this comment

davidgamez Apr 5, 2023

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2023

isabelle-dr commented Apr 12, 2023