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

[SDK-3997] Upload test results to observability server #1551

Merged

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Dec 14, 2023

Generates JUnit format results for the Node and browser tests, and uploads them to the test observability server. Based on work started by @owenpearson in #1009. See commit messages for more details.

Here is an example upload with some failures. (Before merging this PR, I'll delete all of the existing ably-js uploads so as to not skew the results with these example failures.)

Note that the observability server UI doesn't currently display a link to the individual matrix job that caused the upload. This is addressed by the following open PRs:

Resolves #1548.

@github-actions github-actions bot temporarily deployed to staging/pull/1551/features December 14, 2023 13:02 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1551/bundle-report December 14, 2023 13:03 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1551/typedoc December 14, 2023 13:03 Inactive
@lawrence-forooghian lawrence-forooghian marked this pull request as ready for review December 14, 2023 13:31
@lawrence-forooghian lawrence-forooghian marked this pull request as draft December 14, 2023 14:04
@lawrence-forooghian
Copy link
Collaborator Author

Sorry, just realised there's still a TODO here; converting to draft for now.

@lawrence-forooghian lawrence-forooghian force-pushed the 1548-upload-test-results-to-observability-server branch from 056f4c9 to 3c684b3 Compare December 14, 2023 15:27
@github-actions github-actions bot temporarily deployed to staging/pull/1551/features December 14, 2023 15:28 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1551/bundle-report December 14, 2023 15:29 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1551/typedoc December 14, 2023 15:29 Inactive
@lawrence-forooghian lawrence-forooghian marked this pull request as ready for review December 14, 2023 16:36
@lawrence-forooghian lawrence-forooghian requested review from VeskeR and owenpearson and removed request for VeskeR December 14, 2023 16:36
@lawrence-forooghian
Copy link
Collaborator Author

Sorry about that — ready for review now.

@github-actions github-actions bot temporarily deployed to staging/pull/1551/features December 14, 2023 23:32 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1551/bundle-report December 14, 2023 23:33 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1551/typedoc December 14, 2023 23:33 Inactive
@lawrence-forooghian
Copy link
Collaborator Author

lawrence-forooghian commented Dec 14, 2023

I've pushed 0f0dbb3 to fix a couple of mistakes. Will squash it once the PR is approved; I don't want to rebase it at the moment since I've got a couple of other WIP branches that are based on an earlier commit in this PR and I don't want to sever that connection right now.

Copy link
Contributor

@VeskeR VeskeR left a comment

Choose a reason for hiding this comment

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

Integration of mocha-junit-reporter LGTM. Happy to merge after comments are resolved.

Also, any particular reason we don't do the same for react-hooks tests? It seems like vitest has a built-in JUnit reporter too.

webpack.config.js Outdated Show resolved Hide resolved
webpack.config.js Show resolved Hide resolved
test/support/mocha_reporter.js Outdated Show resolved Hide resolved
test/support/runPlaywrightTests.js Outdated Show resolved Hide resolved
owenpearson and others added 3 commits January 10, 2024 12:09
The test reports emitted by mocha-junit-reporter have <testcase> tags
with slightly weird `name` and `classname` properties, e.g.

<testsuite name="rest/defaults">
    <testcase name="rest/defaults Init with given environment" classname="Init with given environment">
    <testcase/>
</testsuite>

This looks a bit odd in the observability server web UI, which displays
the testcase.name and testcase.classname properties. I don’t know
whether it’d be better to do some pre-upload manipulation of the ably-js
reports, or to change the fields displayed in the web UI. Not going to
do anything about it now; it’s usable enough and I don’t want to mess up
the display of the ably-cocoa test reports.

Co-authored-by: Owen Pearson <owen.pearson@ably.com>
Resolves #1548.

Co-authored-by: Owen Pearson <owen.pearson@ably.com>
@lawrence-forooghian
Copy link
Collaborator Author

Also, any particular reason we don't do the same for react-hooks tests?

I didn't even consider it, to be honest. The normal test suite is where we've been seeing the flakiness, as far as I'm aware. Also the observability server doesn't currently support differentiating between different test suites in a given repository.

@VeskeR
Copy link
Contributor

VeskeR commented Jan 10, 2024

The normal test suite is where we've been seeing the flakiness

I see now, seems reasonable. No need for react-hooks tests upload for now I think.

@VeskeR VeskeR self-requested a review January 10, 2024 17:08
@lawrence-forooghian lawrence-forooghian merged commit 584a46b into main Jan 11, 2024
11 checks passed
@lawrence-forooghian lawrence-forooghian deleted the 1548-upload-test-results-to-observability-server branch January 11, 2024 08:45
@lawrence-forooghian lawrence-forooghian mentioned this pull request May 29, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Upload test results to observability server
3 participants