-
Notifications
You must be signed in to change notification settings - Fork 16
fix: improve feedback on results of automated report update #1441
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
Conversation
@stalgiag It would be helpful if you could include some screenshots of the UI updates whenever you get a chance! |
Good suggestion! Comment updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and functions as intended. Thanks for the help in getting this running on my machine, Stalgia!
// Validate each applicable test's results against historical results | ||
for (const test of applicableTests) { | ||
const currResult = testPlanRun.testResults.find( | ||
tr => String(tr.testId) === String(test.id) | ||
); | ||
|
||
if (!currResult) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want to set allOutputsMatch = false
in this case? And if so maybe we can consider this branch an early optimization and just rely on the condition that follows the calculation of histResult
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout! Addressed in e173a1f
addresses #1417
Overview
This PR addresses issues with the automated report update that made it difficult to determine what deviations existed with previous runs. The previous implementation did not "submit" results unless all outputs matched all historical output. This meant that there was no quick way to determine which results differed. This PR changes a few features to make this feedback more clear:
There is one requirement from the linked issue that I did not address, namely the request to list conflicts for update runs. In cases where the output does not match, no verdicts are assigned, therefore there are not yet "conflicts" as they are understood in the app. A human reviewer must review the new, different output and complete the assertion verdicts. At this point, the current conflicts implementation should understand any deviation as conflicts.
Testing
Testing this can be tricky. I have included a guide below to help reviewers. Obviously varied testing is helpful so feel free to disregard this guide once you understand the DB state needed for testing.
Testing scenarios with deviation
For this I recommend running the actual automated runs. This requires setting up a ngrok FQDN as per the deploy README.
Start by restoring the prod dump from the automated runs done previously. This is called
production_dump_20250603.sql
and shared internally.Note that you will need to temporarily delete this migration while running the migration setup step, otherwise the migration will remove the relevant reports.
Once that is complete, you can go to the Test Queue and delete the latest automated "Action Menu Button Example Using aria-activedescendant" report. Refresh and navigate to the "Automated Report Updates" tab. You should see the option to "Start Generating Reports" under NVDA Bot 2024.4.1.
After the job finishes running (~15 minutes), you should see an updated status in the Update Events table that indicates there was 1 difference in the run. When you view the test plan run, the test result for test 3 will show as unsubmitted. This is due to a difference in the third output.
Testing scenarios without deviation
For this, I recommend starting with a standard preseeded DB via the population scripts and not connecting to GH Actions. This will result in the local development simulator automatically putting all of the same output for each test. You can then create a test run with NVDA 2024.1, add NVDA 2024.4.1 to the AT Versions, rerun and see the report automatically finalized.
Screenshots
As mentioned previously, there was no visual or auditory indication when an update rerun had test outputs that did not match the previous report. Changes were made to address this. I am including screenshots below:
Before Screenshots
Test run view, test 3 is selected and shows as unsubmitted in the main content panel with the output and radio buttons for assertion verdicts. Note that all other tests would show as unsubmitted as well. The nav along the lefthand side has a green checkmark for all tests.After Screenshots
Test run view, test 1 is selected and shows as submitted in the main content panel. The nav along the left hand side has a greencheck mark next to the nav link for test 1.Test run view, test 3 is selected and shows as unsubmitted in the main content panel with the output and radio buttons for assertion verdicts. The nav along the left hand side has a white circle in a slightly paler green circle next to the nav link for test 3.
