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

#227: Webpack code coverage support #266

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

gtrombitas
Copy link

@gtrombitas gtrombitas commented Jul 7, 2022

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • New test runner
  • Documentation
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

If you are adding a new test runner, have you...? (check all)

  • Created an issue first?
  • Registered it in /packages/base/runners.json?
  • Added it to /README.md?
  • Included one test that runs baseline.spec.vue?
  • Added and updated documentation?
  • Included a recipe folder with properly building quasar project?

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the dev branch and not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • It's been tested on Windows
  • It's been tested on Linux
  • It's been tested on MacOS
  • Any necessary documentation has been added or updated in the docs (for faster update click on "Suggest an edit on GitHub" at bottom of page) or explained in the PR's description.

If adding a new feature, the PR's description includes:

  • The modification introduces Webpack support for e2e-cypress package.

Other information:

add @cypress/code-coverage according to this link (babel-plugin-istanbul isn't needed here because it's bundled with coverage-istanbul-loader.

add "code-coverage" option to quasar.extensions.json

"@quasar/testing-e2e-cypress": {
    "options": [
      "scripts",
      "typescript",
      "code-coverage"
    ]
  }

add nyc settings in the end of package.json

"nyc": {
    "temp-dir": "temp/.nyc_output",
    "include": [
      "src/**"
    ],
    "extension": [
      ".js",
      ".ts",
      ".vue"
    ],
    "report-dir": "test/cypress/coverage"
  }

execute sync:cypress npm script in test-project-webpack project

// See https://www.npmjs.com/package/istanbul-instrumenter-loader
// https://github.com/vuejs/vue-cli/issues/1363#issuecomment-405352542
// https://github.com/akoidan/vue-webpack-typescript
if (api.hasWebpack === true) {
Copy link
Author

@gtrombitas gtrombitas Jul 7, 2022

Choose a reason for hiding this comment

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

@IlCallo When I've tried then hasWebpack condition was undefined. How can I set it according to the project's setting?

Copy link
Member

Choose a reason for hiding this comment

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

hasWebpack is undefined into q/app projects, but should be defined into all others
We could actually just remove hasWebpack and only rely on hasVite: if hasVite is false or undefined, then the project will automatically be webpack-based

@@ -44,6 +44,7 @@
},
"devDependencies": {
"@types/lodash": "^4.14.178",
"coverage-istanbul-loader": "^3.0.5",
Copy link
Member

Choose a reason for hiding this comment

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

Remember that the Cypress AE devDeps are the ones which are used into dev phase of the app extension, while packages which must be used into userland project must go into the app extension normal dependencies

So this package should be moved to dependencies

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I fixed it.

// See https://www.npmjs.com/package/istanbul-instrumenter-loader
// https://github.com/vuejs/vue-cli/issues/1363#issuecomment-405352542
// https://github.com/akoidan/vue-webpack-typescript
if (api.hasWebpack === true) {
Copy link
Member

Choose a reason for hiding this comment

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

hasWebpack is undefined into q/app projects, but should be defined into all others
We could actually just remove hasWebpack and only rely on hasVite: if hasVite is false or undefined, then the project will automatically be webpack-based

loader: '@jsdevtools/coverage-istanbul-loader',
options: { esModules: true },
enforce: 'post',
include: path.join(__dirname, '..', '..', '..', '..', 'src'),
Copy link
Member

Choose a reason for hiding this comment

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

This seems off, the include (provided the exclude isn't enough on its own) should be relative the the user folder, not to this AE location

Copy link
Author

Choose a reason for hiding this comment

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

I removed include property and I extended exlcude with .quasar folder. Now it seems to be working with this setting.

@IlCallo
Copy link
Member

IlCallo commented Jul 7, 2022

Thanks for the contribution :)
I don't have time to test it right now, but I left some comments on what seems to be off at a first glance

@gtrombitas
Copy link
Author

@IlCallo I fixed the requested changes please check them.

@maiolica
Copy link

maiolica commented Jul 15, 2022

hi @gtrombitas, and thanks for the PR, I have run some tests on it.
Coverage of .ts files is perfect, but .vue files seem to have some issues, it seems all files are getting the exact same coverage, with the same uncovered block position (see screens).

image
image

@gtrombitas
Copy link
Author

gtrombitas commented Jul 19, 2022

@maiolica Thank you for the review and testing effort. Unfortunately I couldn't find any other solutions to handle .vue file instrumentation better.
I opened an issue to coverage-istanbul-loader library: link
Hopefully they can advise a concrete solution or a workaround for this problem.

@IlCallo
Copy link
Member

IlCallo commented Nov 29, 2022

I'm regularly checking that issue, but the project seems abandoned since 2020
The whole Instanbul ecosystem seems freezed in time right now

Is there any reliable alternative into JS ecosystem?

@gtrombitas
Copy link
Author

Thanks for the sync-up. Yes, I have seen the same thing at Istanbul repo.
Unfortunately I'm not familiar with any other alternatives to this library.
When I have a little time for it then I'll try to look into some other libs. If something already exists at all...

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

3 participants