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

Self-test script rewrite #232

Merged
merged 39 commits into from
Dec 27, 2024
Merged

Self-test script rewrite #232

merged 39 commits into from
Dec 27, 2024

Conversation

Luc45
Copy link
Member

@Luc45 Luc45 commented Dec 23, 2024

Background & Motivation

The motivation for this PR came after I wasted a couple of hours while doing the sync with 9.6.0-beta.1. One of the self-tests didn't start, and I only found out about this 2h later. Debugging why it failed was much harder than it should be, because the self-test script would just dispatch 20 tests in parallel, keep them all alive with --wait and --json, where each would poll their own test status individually.

This PR greatly simplifies this setup, we dispatch the tests async and collect their test run IDs, then we poll their status ourselves with one request to the newly introduced get-multiple endpoint, which takes a comma-separated list of test run IDs.

This made it possible to also improve the output, and added extensive logging to all requests to last-self-test.log, that gets reset with each self test.

What Changed: Before vs. After

Before

  • Multiple Parallel Processes: We would start many processes, each:

    1. Dispatching its QIT test.
    2. Polling individually for status until completion.
    3. Reporting success or failure.
  • Drawbacks:

    • Hard to coordinate and debug: each process produced its own logs and status updates.
    • Risk of wasted time: if one test fails to run, it would be very hard to know why.
    • More overhead managing N distinct polling loops concurrently.

After

  1. Asynchronous Dispatch

    • All tests are dispatched quickly (no blocking).
    • We gather every test_run_id upfront.
  2. Unified Polling Loop

    • Rather than multiple pollers, a single loop calls qit get-multiple for all test_run_ids in one go.
    • This repeats every X seconds until each test reports a final status.
  3. Snapshot Verification

    • As soon as a test finishes, its JSON result is stored and checked against the PHPUnit snapshot.
    • Any mismatch shows up right away (or updates, if in update mode).

Implementation Details

  • QitRunner.php:
    • Dispatches all tests asynchronously and records their IDs.
    • Repeatedly calls qit get-multiple until all have a final result.
  • QITLiveOutput.php:
    • Adjusted to show a unified table of test statuses during each poll iteration.
  • PhpUnitRunner.php:
    • Receives completed test data and checks it against snapshots.
  • Documentation:
    • Updated to explain the single polling flow (instead of many parallel pollers).

Testing instructions

  • Run the self-tests as usual, see how it goes

@Luc45 Luc45 self-assigned this Dec 24, 2024
@Luc45 Luc45 requested a review from a team December 24, 2024 01:51
@Luc45 Luc45 marked this pull request as ready for review December 24, 2024 01:51
@zhongruige
Copy link
Contributor

I'll dig into this a bit more tomorrow and run the tests but so far loving these changes!

@Luc45
Copy link
Member Author

Luc45 commented Dec 24, 2024

No worries! Ignore if you see some activity in GH, I'm just taking the opportunity to run some self-tests and scheduling the PC to auto-shutdown in a couple of hours.

@zhongruige
Copy link
Contributor

Went through the self tests (php QITSelfTests.php) and it batched and incremented as it was running:

Screenshot 2024-12-24 at 8 39 49 AM

However, I did hit a timeout and it eventually exited:

Screenshot 2024-12-24 at 12 15 32 PM

Wanted to just double check on the above behavior.

@Luc45
Copy link
Member Author

Luc45 commented Dec 26, 2024

Wanted to just double check on the above behavior.

Nice catch, I've limitted the get-multiple endpoint to 20 tests per request on the backend, and updated the logic here to:

  • Stop requesting tests that are complete
  • Batch polling in chunks of 20 tests, instead of trying to fetch them all at once

I was able to run a mass test with all tests at once, so it seems to have solved it.

@Luc45 Luc45 requested a review from zhongruige December 26, 2024 23:29
@Luc45 Luc45 mentioned this pull request Dec 27, 2024
@zhongruige
Copy link
Contributor

Re-ran this again and just finished (ran this twice just to see) and got this result both times:

Screenshot 2024-12-27 at 9 00 14 AM

Would we expect these all to pass or is this expected as we have some no-op tests we might expect to fail?

@Luc45
Copy link
Member Author

Luc45 commented Dec 27, 2024

It should pass on the 9.6.0-beta.1 sync branch

@zhongruige
Copy link
Contributor

Weird I got the same results on that branch @Luc45:

All snapshots have been verified.

Some snapshots still failed. Final outcome: ❌

For more details, see last-self-test.log.
➜  managed_tests git:(24-12/sync-960) 

Not sure, could it be due to the PHP version I'm using?

@zhongruige
Copy link
Contributor

Perfect that fixed it, thanks @Luc45!

Screenshot 2024-12-27 at 12 47 19 PM

@Luc45 Luc45 merged commit e0d15a1 into trunk Dec 27, 2024
@Luc45 Luc45 deleted the 24-12/self-test-script-rewrite branch December 27, 2024 19:54
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