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

Some small e2e tests for report viewer #1280

Merged
merged 28 commits into from
Jan 22, 2024
Merged

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented Sep 6, 2023

This PR adds some small tests for the basic functionality of each page

@tsaglam tsaglam added enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Sep 26, 2023
@tsaglam tsaglam added this to the v5.0.0 milestone Oct 4, 2023
@sonarcloud
Copy link

sonarcloud bot commented Oct 6, 2023

[JPlag Report Viewer] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Kr0nox Kr0nox requested a review from sebinside October 6, 2023 12:42
@Kr0nox
Copy link
Member Author

Kr0nox commented Oct 6, 2023

@sebinside This PR is now ready for review.
Snapshots are added for windows and linux systems (although font faces and other criteria can be different).
The html report gets automatically added to the artifacts of the action run

@Kr0nox
Copy link
Member Author

Kr0nox commented Oct 7, 2023

The tests will need to be changed either in this PR after merging #1323. So merge #1323 first.

Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

At least for me, the tests fail locally with

Test timeout of 30000ms exceeded.
Error: page.waitForEvent: Page closed
waiting for event "filechooser"

but that could also be a configuration thing, playwright is not exactly lightweight :)

Other than this we should discuss which standards we want regarding code quality and comprehensiveness of these test cases in the next meeting. I see the benefit of direct feedback by the CI/CD pipeline if we break something. However, this can quickly develop into something hard to maintain and hard to keep up-to-date.

Copy link

sonarcloud bot commented Dec 8, 2023

[JPlag Report Viewer] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Kr0nox
Copy link
Member Author

Kr0nox commented Dec 8, 2023

After merging the develop branch into this and trying to adapt the tests, I agree with @sebinside that the current construct is hard to maintain at times.

In hindsight comparing screenshots should probably only be done for the entire page or elements with a constant size. Dynamically sized elements can have a different size on different machines. In addition I could not provide schreenshots for mac users, so this is another challange. I am against screenshotting the entire page, since it changes so rapidly, that every PR would update 20 images.

Instead I opted for not doing a check for the cluster chart. For the distribution and the radar chart, we check, that the diagramm changes when it is supposed to. (change similarity, ...)

@Kr0nox
Copy link
Member Author

Kr0nox commented Jan 17, 2024

Before merging this, develop should be merged into this branch again, even if there are no merge conflicts, to ensure the tests are all still correct

Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

Good simplification, it now even works locally for me :)

@Kr0nox: please merge the current version so that we can finish this PR.

Copy link

sonarcloud bot commented Jan 22, 2024

Quality Gate Passed Quality Gate passed for 'JPlag Report Viewer'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Kr0nox
Copy link
Member Author

Kr0nox commented Jan 22, 2024

@sebinside Merged and updated the test that were failing. Can be merged from my side

@sebinside sebinside merged commit 426b2a1 into develop Jan 22, 2024
20 checks passed
@sebinside sebinside deleted the report-viewer/simple-e2e branch January 22, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants