Skip to content

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

Merged
merged 8 commits into from
Jun 25, 2025

Conversation

stalgiag
Copy link
Contributor

@stalgiag stalgiag commented Jun 20, 2025

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:

  1. Each individual test result that has matching output with the previous report will be submitted
  2. Any test that has differing output in a scenario will not be submitted and the test navigator for automated jobs now distinguishes between jobs that have been submitted and jobs that have not.
  3. Add update to the status table on the automated report updates tab that communicates whether an update was completed with or without differences.
  4. Keep the "Manage Bot" dialog available after a job has completed to make reassigning options 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.

@stalgiag stalgiag requested review from jugglinmike and ChrisC June 20, 2025 19:25
@ChrisC
Copy link
Contributor

ChrisC commented Jun 23, 2025

@stalgiag It would be helpful if you could include some screenshots of the UI updates whenever you get a chance!

@stalgiag
Copy link
Contributor Author

@stalgiag It would be helpful if you could include some screenshots of the UI updates whenever you get a chance!

Good suggestion! Comment updated.

Copy link
Contributor

@jugglinmike jugglinmike left a 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;
Copy link
Contributor

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.

Copy link
Contributor Author

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

@stalgiag stalgiag merged commit 511a73a into development Jun 25, 2025
1 of 3 checks passed
@stalgiag stalgiag deleted the update-rerun-fix branch June 25, 2025 22:10
howard-e added a commit that referenced this pull request Jul 1, 2025
Create July 1, 2025 Release

Includes the following changes:

* #1428, which addresses #1410
* #1441, which addresses #1417
* #1443, which addresses #1442
* #1440, which addresses #1429
* #1445, which addresses #1400
* #1438, which addresses #1427
* #1446, which addresses #1430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants