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

Karma: use a single webpack bundle for all tests #2887

Merged
merged 5 commits into from
Jul 1, 2020
Merged

Conversation

dvoytenko
Copy link
Contributor

@dvoytenko dvoytenko commented Jul 1, 2020

Summary

Create a single webpack bundle for all Karma tests per project.

Relevant Technical Choices

The initial configuration was done per test file. The alternative, instead recommends to combining all tests into a single bundle. This requires a single "weird" karma-tests.cjs file to combine all tests together (sort of like import-all-by-mask). However, this has a lot of performance benefits. The bundling is about 3x faster and memory requirements are significantly lower.

There's one small drawback: the webpack import-all-by-mask regex can only work on a file name and not the full path. So, all karma test files should be named *.karma.js. In practice this was the case for all but 2 files.

And there's one more nuance. For some reason, the old config was running some tests a few times. I guess somehow the related test files were being bundled together and repeated. Thus, before this pull request, the test run was reporting 105 tests and now it will report 83. I double-checked in the source tree, and indeed, we only have 83 Karma tests total. Thus the new numbers are correct and the old ones are wrong. This doesn't, of course, change the coverage since rerunning the same test twice doesn't add or remove covered lines.

User-facing changes

n/a

Testing Instructions

n/a


Fixes #2851

@codecov
Copy link

codecov bot commented Jul 1, 2020

Codecov Report

Merging #2887 into master will increase coverage by 12.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #2887       +/-   ##
===========================================
+ Coverage   69.23%   81.28%   +12.04%     
===========================================
  Files         699      701        +2     
  Lines       11878    11908       +30     
===========================================
+ Hits         8224     9679     +1455     
+ Misses       3654     2229     -1425     
Flag Coverage Δ
#karmatests 59.72% <ø> (+38.61%) ⬆️
#unittests 66.44% <ø> (-0.13%) ⬇️
Impacted Files Coverage Δ
...ponents/library/panes/media/media3p/media3pPane.js 11.76% <0.00%> (-54.91%) ⬇️
.../src/dashboard/app/views/myStories/header/index.js 84.61% <0.00%> (-4.28%) ⬇️
...sets/src/dashboard/app/views/shared/pageHeading.js 100.00% <0.00%> (ø)
...ory/components/library/panes/media/mediaGallery.js 33.33% <0.00%> (ø)
...ory/components/library/panes/media/providerType.js 100.00% <0.00%> (ø)
...s/src/edit-story/components/panels/sizePosition.js 92.77% <0.00%> (+2.40%) ⬆️
assets/src/edit-story/elements/text/frame.js 97.50% <0.00%> (+2.50%) ⬆️
...edit-story/components/panels/panel/shared/title.js 77.41% <0.00%> (+3.22%) ⬆️
assets/src/edit-story/components/form/media.js 85.71% <0.00%> (+3.57%) ⬆️
assets/src/edit-story/components/button/index.js 97.22% <0.00%> (+4.16%) ⬆️
... and 127 more

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2020

Size Change: +6.44 kB (0%)

Total Size: 1 MB

Filename Size Change
assets/js/edit-story.js 457 kB +3.54 kB (0%)
assets/js/stories-dashboard.js 527 kB +2.9 kB (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 269 B 0 B
assets/css/stories-dashboard.css 305 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/web-stories-embed-block.js 15.7 kB 0 B

compressed-size-action

@dvoytenko dvoytenko marked this pull request as ready for review July 1, 2020 01:04
Copy link
Contributor

@dmmulroy dmmulroy left a comment

Choose a reason for hiding this comment

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

lgtm. Once this is merged I'll update my current branch to reflect the changes

@swissspidy swissspidy added the Type: Infrastructure Changes impacting testing infrastructure or build tooling label Jul 1, 2020
@swissspidy swissspidy changed the title Karma: use a single WebPack bundle for all tests Karma: use a single webpack bundle for all tests Jul 1, 2020
@swissspidy swissspidy merged commit e1ccd09 into master Jul 1, 2020
@swissspidy swissspidy deleted the integr/mem branch July 1, 2020 10:14
swissspidy added a commit that referenced this pull request Jan 4, 2021
Only the 5.0.0-alpha version is compatible with webpack 5, and it does not support the alternative usage we’ve been using anymore.

Reverts #2887 because of that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix karma tests memory issues
4 participants