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

Add crypto.randomBytes #922

Merged
merged 7 commits into from
Feb 13, 2019
Merged

Add crypto.randomBytes #922

merged 7 commits into from
Feb 13, 2019

Conversation

bookmoons
Copy link
Contributor

Adds method randomBytes to k6/crypto. The method takes a single parameter size specifying byte count and returns an array of random bytes. Bytes are represented as number literals.

Toward #900.

@codecov-io
Copy link

codecov-io commented Feb 10, 2019

Codecov Report

Merging #922 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
+ Coverage   69.77%   69.82%   +0.05%     
==========================================
  Files         112      112              
  Lines        8815     8823       +8     
==========================================
+ Hits         6151     6161      +10     
+ Misses       2262     2261       -1     
+ Partials      402      401       -1
Impacted Files Coverage Δ
js/modules/k6/crypto/crypto.go 97.47% <100%> (+0.18%) ⬆️
core/engine.go 93.92% <0%> (+0.93%) ⬆️

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 9af59bc...505274c. Read the comment docs.

@bookmoons
Copy link
Contributor Author

AppVeyor failed on this, but it seems to be from a different part of the code. I don't think it's from the changes:

--- FAIL: TestRealTimeAndSetupTeardownMetrics (5.97s)

@bookmoons
Copy link
Contributor Author

Codecov is showing 95.72%, but running locally I get 96.1%. Is it possible it doesn't rerun if the CI builds fail?

@mstoykov
Copy link
Contributor

LGTM!
That test is known to be problematic #919 and I am thinking of disabiling it for now on appveyor as it mostly fails for no reason. On the point of coverage - I think codecov doesn't update when only files it doesn't track are changed and in this case only test are changed ... which is also not ideal :(

@bookmoons
Copy link
Contributor Author

Should I be updating the copyright date on files I modify? It's a couple years old now.

@mstoykov
Copy link
Contributor

I am pretty sure we should make some script or use some tool to update this automatically ...

Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

LGTM

@mstoykov mstoykov merged commit fb4d0ce into grafana:master Feb 13, 2019
@bookmoons
Copy link
Contributor Author

Thanks for merging guys.

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