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

TINY-11177: Vastly improve remote testing #145

Draft
wants to merge 53 commits into
base: master
Choose a base branch
from

Conversation

TheSpyder
Copy link
Member

Related Ticket: TINY-11177

Description of Changes:

  • Remote testing was slow because bedrock sent session POST requests to /start before each it block, and then again to /results after each it block. It would wait for these requests to finish before continuing.
  • /start is now sent at startup and after every 50 tests. This can be made larger if we find it's still too slow.
  • /results is now only sent for failure and skip, not pass.
  • The bedrock UI doesn't block on these status requests anymore, it just throws them into an array and uses Promise.all() at the very end to wait for them.
  • Unfortunately the bedrock CLI console HUD depended on receiving these status updates, so I had to adjust it to account for the missing reports.
  • The bedrock HTML page now has a fake icon to remove the favicon.ico request which probably also slowed things down.
  • A proper keep-alive header is now set, instead of using the default 5 seconds which was causing occasional 502 Bad Gateway errors.
  • Reverted TINY-10708: Suppress bedrock logs in remote testing #142 which was designed to solve this problem but server-side was the wrong answer.

Pre-checks:

  • Changelog entry added
  • package.json versions have not been changed (done by Lerna on release)
  • Tests have been added (if applicable)

Before merging:

  • Ensure internal dependencies are on appropriate versions
    • For stable releases, all dependencies must be stable
    • For release candidates, all dependencies must be release candidates or stable

@TheSpyder TheSpyder requested a review from tjdett August 16, 2024 06:37
@TheSpyder TheSpyder requested a review from a team as a code owner August 16, 2024 06:37
Copy link
Member

@spocke spocke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this affect junit reporting would the in flight requests make them in random order or would things be skipped.

@TheSpyder
Copy link
Member Author

I posted in slack that this destroys JUnit reporting. Ideas for how to fix:
https://cksource.slack.com/archives/C04KXE6QUTX/p1724022794632699?thread_ts=1723766938.410059&cid=C04KXE6QUTX

@TheSpyder TheSpyder marked this pull request as draft August 19, 2024 00:05
Jenkinsfile Outdated Show resolved Hide resolved
modules/runner/src/main/ts/reporter/Reporter.ts Outdated Show resolved Hide resolved
@ltrouton ltrouton self-requested a review September 17, 2024 23:23
TheSpyder and others added 18 commits December 4, 2024 11:13
…it for the report AJAX requests to complete before moving on to the next test.
…it would be misleading if they were treated as passing.
…ts. Manual mode works, auto still needs some work.
…to the end of the test run are now waited for correctly.
@jscasca jscasca force-pushed the feature/TINY-11177 branch from 8afd4d5 to 40151ea Compare December 4, 2024 00:38
@TheSpyder
Copy link
Member Author

For the record, JUnit records are long since fixed.

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.

4 participants