fix: code coverage incomplete on redirect by fvclaus · Pull Request #660 · cypress-io/code-coverage · GitHub
Skip to content

fix: code coverage incomplete on redirect#660

Merged
emilyrohrbough merged 15 commits into
cypress-io:masterfrom
fvclaus:fix/coverage-on-application-reload
Oct 25, 2023
Merged

fix: code coverage incomplete on redirect#660
emilyrohrbough merged 15 commits into
cypress-io:masterfrom
fvclaus:fix/coverage-on-application-reload

Conversation

@fvclaus

@fvclaus fvclaus commented May 26, 2023

Copy link
Copy Markdown
Contributor

We have a frontend that, upon loading, redirects the user to Keycloak. The user logs in and is returned to the application. The code coverage written to the disk was incomplete and after hours of debugging I found the issue. The problem is the caching of references to the window.__coverage__ object. When the code that redirects the user is instrumented, the covered statements will be different for the initial redirect and the subsequent return so this plugin will correctly recognize them as different and will keep a reference to both coverage objects. When the code that redirects the user is not instrumented (because it is a library), the coverage object will be identical. Both coverage objects will have identical statement coverage and the current version of the code will throw away the second coverage object, keeping only the reference to the first one which is not updated anymore. This will result in a code coverage where only the statments until the call to the library are covered.

Before:

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |      50 |      100 |       0 |      50 |                   
 utils.js |      50 |      100 |       0 |      50 | 2                 
----------|---------|----------|---------|---------|-------------------
ERROR: Coverage for lines (50%) does not meet global threshold (100%)

After:

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |                   
 utils.js |     100 |      100 |     100 |     100 |                   
----------|---------|----------|---------|---------|-------------------

Keywords for Google: Code coverage different in afterEach.

@CLAassistant

CLAassistant commented May 26, 2023

Copy link
Copy Markdown

Comment thread test-apps/new-cypress-config/returning-to-application/app.js Outdated
Comment thread .nycrc.json Outdated
"support-utils.js",
"task-utils.js"
"task-utils.js",
"support.js"

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.

Why is this being excluded?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you run the unit-tests, support.js is reported as having too little coverage, but support.js is not even being tested (only support-utils.js). I thought that this is an issue with my code, but I get the same result on the main branch.

Comment thread support.js
Co-authored-by: Matt Schile <mschile@gmail.com>
@mschile mschile assigned cacieprins and unassigned mschile Jun 23, 2023
@fvclaus

fvclaus commented Sep 18, 2023

Copy link
Copy Markdown
Contributor Author

@mschile , @cacieprins This MR has been hanging around for almost 4 months. Is there something I can do to get this merged?

@mschile

mschile commented Oct 13, 2023

Copy link
Copy Markdown
Contributor

Hi @fvclaus 👋, sorry for the delay. I will try to get back to this very soon.

@mschile mschile assigned mschile and unassigned cacieprins Oct 13, 2023
@mschile mschile changed the title Coverage incomplete on redirect / reload fix: code coverage incomplete on redirect Oct 23, 2023

@emilyrohrbough emilyrohrbough left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@fvclaus Nice job tracking down the root-cause of this issues. We appreciate the contribution!

@emilyrohrbough emilyrohrbough merged commit 443029b into cypress-io:master Oct 25, 2023
@cypress-app-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants