Skip to content

[FIX] : Web: base test health on finished master runs - #1246

Open
pulk17 wants to merge 1 commit into
CCExtractor:masterfrom
pulk17:web-test-history
Open

pulk17 wants to merge 1 commit into
CCExtractor:masterfrom
pulk17:web-test-history

Conversation

@pulk17

@pulk17 pulk17 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Please prefix your pull request with one of the following: [FEATURE] [FIX] [IMPROVEMENT].

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have never used the project.
  • I have used the project briefly.
  • I have used the project extensively, but have not contributed previously.
  • I am an active contributor to the project.

Problem

On /app/tests every test says "no runs yet", although they do have runs, and the "failing" toggle shows nothing.

Two bugs in how each test's history was built, both from the original Web-Api code:

  1. Master runs were picked out of the shared "newest 40 runs" list. With the queue stuck (fix(ci): keep the cron queue moving when one test fails to start #1244), those 40 are all PR runs, so no master runs were left and every strip was empty. Queued or stuck runs also counted, though they have no results.
  2. Linux and Windows were merged into one series, and "currently failing" read the last entry. That entry was whichever platform had the higher run id. On the latest master commit, tests Fixes creation of blank table divisions. #29 and Update the platform to compare against the latest commit from the master branch #31 fail on Linux but pass on Windows, so they showed as passing and were missing from the failing filter.

Fix

  • History asks for master runs directly: /runs?branch=master&repository=CCExtractor/ccextractor. PR runs are stored on branch pull_request, and fork pushes under their own repository, so neither can crowd it out. Only finished runs (pass / fail) are used.
  • History is folded per commit, over the last 6. A test fails a commit if it failed on either platform.
  • The detail panel lists every run of the test, PR runs included, which is why it disagreed with the list. PR runs there now carry a PR tag, and the caption says so.

Testing

Against a copy of the production database (data up to June):

  • I wrote a separate script that works out from the API which tests are failing on master. It found 8 (18, 21, 29, 31, 37, 136, 162, 225) out of 212 tests with history.
  • The console agrees exactly: the rail shows 8 failing of 237, the failing toggle lists the same 8 ids, and the 25 tests without master history are the ones showing "no runs yet".
  • The network log shows history now comes from the dedicated master query.
  • tsc -b, oxlint --deny-warnings (pinned and latest) and build:app all pass.

That data predates the stuck queue, so the empty list itself isn't reproducible locally. The fix removes the dependency on the shared list, so it no longer matters what's in it.

@knowprasad

Copy link
Copy Markdown

Took a look through \web/src/lib/api.ts. Querying \MASTER_RUNS\ directly makes a lot of sense so PR runs don't clutter the test history. Also nice catch with \ oldCommitRows\ checking both platforms per commit so failures don't get hidden. Checks look all green on my end too.

the list picked master runs out of the newest 40, so it went blank whenever pr runs filled them, and it read whichever platform ran last, so a linux-only failure could show as passing. pr runs are now labelled in the detail panel.
@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants