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

Retouching how files are handled when running from archive fixes #819 #823

Merged
merged 2 commits into from Oct 29, 2018

Conversation

mstoykov
Copy link
Collaborator

Before this commit running an archive meant that all the files will be
loaded and cached before execution. This also included precompiling all
the javascript files without actually running any of them.
This in itself wasn't a problem the problem came from anonymazation of
usernames in directories. This when the paths are absolute meant that we
had to change the script as well in order for them to match - we both
didn't do it and it wouldn't have worked in all cases as they could be
made concatinating strings for example.

The solution - either remove anonymization or anonymize everything
coming trough open. The second solution was choosen for now.

This commit also makes it so that we have an afero.Fs when running from
archive that is populated with all the files from the archive with their
correct anonymized paths. This prevents the panic that was previously
happening.

Also now we just compile and run the test script and it will
automatically load whatever scripts it need from the afero.fs the same
way as this is when just using 'k6 run test.js'

Before this commit running an archive meant that all the files will be
loaded and cached before execution. This also included precompiling all
the javascript files without actually running any of them.
This in itself wasn't a problem the problem came from anonymazation of
usernames in directories. This when the paths are absolute meant that we
had to change the script as well in order for them to match - we both
didn't do it and it wouldn't have worked in all cases as they could be
made concatinating strings for example.

The solution - either remove anonymization or anonymize everything
coming trough open. The second solution was choosen for now.

This commit also makes it so that we have an afero.Fs when running from
archive that is populated with all the files from the archive with their
correct anonymized paths. This prevents the panic that was previously
happening.

Also now we just compile and run the test script and it will
automatically load whatever scripts it need from the afero.fs the same
way as this is when just using 'k6 run test.js'
Copy link
Member

@robingustafsson robingustafsson left a comment

Choose a reason for hiding this comment

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

Nice @mstoykov! LGTM. There's a spelling error that the CircleCI lint job is complaining about though :)

@codecov-io
Copy link

Codecov Report

Merging #823 into master will increase coverage by 0.03%.
The diff coverage is 78.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #823      +/-   ##
==========================================
+ Coverage    69.9%   69.94%   +0.03%     
==========================================
  Files         111      111              
  Lines        8683     8690       +7     
==========================================
+ Hits         6070     6078       +8     
  Misses       2214     2214              
+ Partials      399      398       -1
Impacted Files Coverage Δ
js/runner.go 87.17% <100%> (+1.7%) ⬆️
js/initcontext.go 100% <100%> (ø) ⬆️
js/bundle.go 83.33% <71.42%> (+1.24%) ⬆️
lib/archive.go 87.07% <75%> (-0.7%) ⬇️
core/engine.go 92.05% <0%> (-0.94%) ⬇️
core/local/local.go 90.81% <0%> (-0.35%) ⬇️

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 5788bee...8b27ee6. Read the comment docs.

@mstoykov mstoykov merged commit 248695a into master Oct 29, 2018
@mstoykov mstoykov deleted the dontAnonymizeArchivesAndOtherFixes branch November 2, 2018 09:12
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