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

Fixup shebang loader order #196

Merged
merged 5 commits into from
Dec 31, 2018
Merged

Fixup shebang loader order #196

merged 5 commits into from
Dec 31, 2018

Conversation

guybedford
Copy link
Contributor

This ensures that analysis can still be correctly applied alongside the shebang loader, fixing the error in #192.

@guybedford
Copy link
Contributor Author

I've also updated the Webpack fixtures here, as it seems the output module numbering has been changed by new work on the v5 branch.

@guybedford
Copy link
Contributor Author

This is quite odd, the unit tests were passing with the updates in the commit there, but now seem to have stopped working again. I'm not sure why the webpack output keeps changing like this perhaps @sokra has an idea? The tests are passing fine for me locally with this yarn.lock.

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

No idea. Do the tests run with the self-build version?

src/index.js Outdated
@@ -139,6 +139,7 @@ module.exports = (
},
{
parser: { amd: false },
test: /^(.(?!.*\.node$))*$/,
Copy link
Member

Choose a reason for hiding this comment

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

That's probably the regexp with the worst performance you could have chosen.

Try: exclude: /\.node$/

@guybedford
Copy link
Contributor Author

Ahh, the issue here was that there are two sets of unit tests - one with and without coverage. Completely forgot, despite being responsible for this :P

@codecov-io
Copy link

Codecov Report

Merging #196 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #196   +/-   ##
=======================================
  Coverage   67.87%   67.87%           
=======================================
  Files          12       12           
  Lines         579      579           
=======================================
  Hits          393      393           
  Misses        186      186
Impacted Files Coverage Δ
src/index.js 70.96% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69b6031...672bc21. Read the comment docs.

@rauchg rauchg merged commit 0b8395e into master Dec 31, 2018
@rauchg rauchg deleted the shebang-analysis branch December 31, 2018 18:35
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.

4 participants