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

Improve test coverage through integration tests #226

Merged
merged 11 commits into from
Jul 4, 2024
Merged

Improve test coverage through integration tests #226

merged 11 commits into from
Jul 4, 2024

Conversation

trieloff
Copy link
Contributor

@trieloff trieloff commented Jul 3, 2024

This has been a bit more painful than I expected.
The integration test now loads helix-rum-enhancer through
a (slightly prepared) version of helix-rum-js and then
validates the checkpoints that have been sent using a mocked
navigator.sendBeacon. This helps to bring up the coverage
of index.js, but there are still good chunks of it that
require more fiddling, especially when we look at URL parameter
comprehension

  • test(index): click somewhere
  • test(integration): perform integration tests with slightly patched rum-js
  • test(webkit): make integration test pass in webkit
  • test(integration): make sure correct code coverage for integration test gets generated
  • test(integration): remove unused plugins
  • test(integration): fake more sendBeacons
  • test(integration): test more checkpoints

@adobe-bot
Copy link

This PR will trigger no release when merged.

@trieloff trieloff requested review from kptdobe and ekremney July 3, 2024 21:00
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.58%. Comparing base (bb71ae6) to head (baaf738).
Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #226       +/-   ##
===========================================
+ Coverage   73.83%   84.58%   +10.75%     
===========================================
  Files           5        5               
  Lines         493      493               
===========================================
+ Hits          364      417       +53     
+ Misses        129       76       -53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@trieloff trieloff mentioned this pull request Jul 3, 2024
2 tasks
<img src="/test/it/img.test.html">
<script type="module">
import { runTests } from '@web/test-runner-mocha';
import { sendMouse } from '@web/test-runner-commands';
Copy link
Member

Choose a reason for hiding this comment

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

@kptdobe referring to the discussion we had back in rumathon, maybe this sendMouse command what we needed for the INP test?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be tested, yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The challenge may be that we have to navigate away from the page and that can interfere with recording the test results.

kptdobe
kptdobe previously approved these changes Jul 4, 2024
Copy link
Contributor

@kptdobe kptdobe left a comment

Choose a reason for hiding this comment

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

Nice!

<img src="/test/it/img.test.html">
<script type="module">
import { runTests } from '@web/test-runner-mocha';
import { sendMouse } from '@web/test-runner-commands';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { sendMouse } from '@web/test-runner-commands';

Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed, no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You got me. I'm clicking now.

<img src="/test/it/img.test.html">
<script type="module">
import { runTests } from '@web/test-runner-mocha';
import { sendMouse } from '@web/test-runner-commands';
Copy link
Contributor

Choose a reason for hiding this comment

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

To be tested, yes.

@trieloff trieloff merged commit 8f2ee8d into main Jul 4, 2024
7 checks passed
@trieloff trieloff deleted the more-tests branch July 4, 2024 11:37
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.

None yet

4 participants