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

Update mocha, chai for testomat reporting internal #21400

Merged
merged 40 commits into from
Nov 13, 2023

Conversation

achakko
Copy link
Contributor

@achakko achakko commented Oct 12, 2023

Description:

This is a pull request to update mocha, chai version for all tests, thereby to help enable CI integration with testomat.io to understand reporting on tests including flakiness

Review

@achakko achakko requested a review from a team October 12, 2023 19:35
@achakko achakko self-assigned this Oct 12, 2023
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Seems you have commited a couple changes to the submodules. This for sure shouldn't be included in this PR and currently causes a lot tests to fail.
You may need to revert those changes. If you need help with git or how to handle submodules feel free to get in touch with me or another dev on slack

* added testomatio reporter for ui tests

* removed api key from code

---------

Co-authored-by: achakko <[email protected]>
@achakko achakko force-pushed the dev-17222-testomat-internal branch from 089e585 to 09f2338 Compare October 15, 2023 16:59
@achakko
Copy link
Contributor Author

achakko commented Oct 15, 2023

Hi @sgiehl @michalkleiner I rebased to remove the commit that was not required. Thanks.

@achakko achakko requested a review from a team October 15, 2023 23:29
@achakko achakko added Needs Review PRs that need a code review Do not close PRs with this label won't be marked as stale by the Close Stale Issues action labels Oct 15, 2023
tests/lib/screenshot-testing/run-tests.js Outdated Show resolved Hide resolved
tests/lib/screenshot-testing/package-lock.json Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added Stale The label used by the Close Stale Issues action and removed Stale The label used by the Close Stale Issues action labels Oct 27, 2023
@sgiehl sgiehl changed the title Dev-17222 Pull request to update mocha, chai for testomat reporting internal Update mocha, chai for testomat reporting internal Oct 30, 2023
@achakko
Copy link
Contributor Author

achakko commented Nov 2, 2023

@matomo-org/core-reviewers This reporting seems to be working now. The CI job ran tests and reported the UI test run results into testomat with a decent stack trace for failed tests as well. Runs can be accessed here. Once we have more data, will get meaningful results in Analytics.
https://app.testomat.io/projects/core-ui-tests/runs
image

Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

The test tomato reporting is looking great 👍

Is there any reason the UI test output log has lots of [TESTOMATIO] Warning: Test could not be matched (404) should be possible to publish new version messages? This will make it a bit harder to check the logs, it also looks like colour highlighting has also disappeared which is helpful to quickly identify the failed tests.

@achakko
Copy link
Contributor Author

achakko commented Nov 9, 2023

The test tomato reporting is looking great 👍
Is there any reason the UI test output log has lots of [TESTOMATIO] Warning: Test could not be matched (404) should be possible to publish new version messages? This will make it a bit harder to check the logs, it also looks like colour highlighting has also disappeared which is helpful to quickly identify the failed tests.

Hi @bx80 I was able to remove the warnings. See one of the recent runs here. Do you think the absence of the colour highlighting is a deal breaker?

Added color = 1 and removed the deprecated Usecolors method. Seems to be working! We have a summary of any failed just above the Error message with colors ofcourse. Good to approve?
image

Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

Thanks @achakko, much easier to read now 👍
Might be good to get additional approvals for this one, as I've not been following all the other points raised.

tests/UI/config.dist.js Outdated Show resolved Hide resolved
@achakko
Copy link
Contributor Author

achakko commented Nov 10, 2023

The issue with the double reporting in testomat.io is logged here. I will follow up for resolution. testomatio/reporter#245

@achakko achakko requested review from mneudert and sgiehl November 12, 2023 21:38
@sgiehl sgiehl mentioned this pull request Nov 13, 2023
11 tasks
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

This overall looks good to me now. Don't have access to the testomatio dashboard, so can't say if it's working on that side.

@sgiehl sgiehl added this to the 5.0.0 milestone Nov 13, 2023
@sgiehl sgiehl merged commit 6d10dce into 5.x-dev Nov 13, 2023
20 of 25 checks passed
@sgiehl sgiehl deleted the dev-17222-testomat-internal branch November 13, 2023 14:46
@sgiehl sgiehl added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do not close PRs with this label won't be marked as stale by the Close Stale Issues action Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants