Skip to content

Drone: readable test report with a permalink test status page url#64786

Merged
snickell merged 14 commits into
stagingfrom
seth/upload-ui-status-html-to-s3
Mar 27, 2025
Merged

Drone: readable test report with a permalink test status page url#64786
snickell merged 14 commits into
stagingfrom
seth/upload-ui-status-html-to-s3

Conversation

@snickell

@snickell snickell commented Mar 25, 2025

Copy link
Copy Markdown
Collaborator

Notes to reviewers: I'm most interested in getting attention to what sort of output / dev-ux this PR provides, and less on details of the code, which I feel reasonably confident in (tho lmk if its busted lol!). No S3 policy/perms changes were required to accomplish this PR.

PR Goals

In my previous PR #63324 on this topic, I improved #infra-test Slack output for the DOTD. This PR builds on that PR with a change of focus: this time the focus is on making it easier for devs to scan the output of UI test failures on drone.

The primary challenge in this PR was balancing "dotd slack formatting needs" against "drone formatting needs". I believe the output in this PR improves both.

The two primary improvements target drone:

  1. The final UI TEST REPORT is MUCH easier to read in the drone build log, and provides an easily searchable string.
  2. The "test status page" feature for DOTD, i.e.https://test-studio.code.org/ui_test/test_status_UI.html, is now a drone feature too 🎉, hosted as static HTML on S3 (real example from a drone run, showing one test failing)

Secondary improvements affect slack and local dev:

  1. Slack output in both PASS and FAIL conditions takes up more vertical space, which really helps it stand against the remaining noise. Its even easier to tell at a glance of #infra-test whether eyes/dashboard or both passed/failed.
  2. Currently the "test status page" URL is ephemeral, as soon as you start a new build, the old set of failures/passes is lost to time. We now provide a second "permalink" URL to an S3 bucket with a "test status page" that is specific to that test run. This is not generally as useful for DOTD, but allows people to link in slack convos in a way that can be viewed later.
  3. improving drone "final test report" output has the same effect on local console ./runner.rb output

Files this PR Modifies

  1. runner.rb: significant rework of the final "report" output, including adding the new S3 test status link
  2. runner.rb: upload test status page to S3, in the same bucket+path as we already upload per-feature cucumber html
  3. test_status.js: if you can't reach test_logs_controller.rb, fallback to doing direct HEAD requests to S3, fetching the same data slightly slower. This allows test_status_.html to be hosted as static HTML on S3*. No S3 policy changes are required, cors works because the html is served off the same domain as the html its doing HEAD requests to get build status.

Before/After shots

When Tests Fail: Drone, Before this PR

image

When Tests Fail: Drone, After this PR

image

If you'd like to try it, here's the UI Test Status Page (permalink for this run), accessible from within drone 🎉: https://cucumber-logs.s3.amazonaws.com/circle/27651/test_status_UI.html?versionId=NHH_tZqVmG2FgdpHAMr2I1Lxe2KAPzVP

When Tests Fail: Slack, Before this PR

My critique of before: short is nice, but it gets lost in the noise a bit.

image

When Tests Fail: Slack, After this PR

image

When Tests Pass: Slack, Before this PR

My critique of before: its not super clear that what completed was just Eyes tests, not "all tests", and while the pass being very terse seemed like an advantage in the past, my experience has been that the eyes pass gets lost in the noise as a result.

image

When Tests Pass: Slack, After this PR

image

* status string ('pass', 'fail', 'in-progress', 'not-started').
* Defaults to gray (not started).
*/
function setTabStatusIcon(typeOrColor) {

@snickell snickell Mar 27, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This function goes to the end of the file and is imported from dashboard/test/ui/js/set-tab-status-icon.js, which is now deleted. Its not new code, reviewers can skip it.

Why move the code here? Having a file in a sub-path made the S3 upload messy. Aaaaaaand: this wasn't a module we were ever going to re-use, its pretty short and is still well encapsulated, there's no build-system or webpack for this anyway, and it was the only file in dashboard/test/ui/js so it removed a one-file-folder.

async function fetchMetadataFromDashboardAPI(since) {
const res = await fetch(`${API_BASEPATH}/${S3_PREFIX}/since/${since}`, {
mode: "no-cors",
});

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This function was refactored out of refresh() below

return metadata
}

async function refresh() {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

refresh() has a relatively large diff change because I reworked it as an async function rather than continuing to chain .then .catch promises. This made the "from controller or direct s3" logic much simpler to follow.

@snickell snickell changed the title Upload test_status_UI.html to S3, serve from a static bucket without TestLogsController Drone: readable test report with a permalink test status page url Mar 27, 2025
@snickell snickell marked this pull request as ready for review March 27, 2025 08:13
@snickell snickell requested a review from breville March 27, 2025 08:18

@molly-moen molly-moen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am trusting your code works 😄
The output is great 😍 Having a link to the status page for drone is going to be hugely helpful. Also having the link to the page in a clear spot is slack is great, that's always the thing I have to scroll to find.

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