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

Benchmark ci fixes #840

Merged
merged 6 commits into from
Aug 25, 2020
Merged

Benchmark ci fixes #840

merged 6 commits into from
Aug 25, 2020

Conversation

ryanio
Copy link
Contributor

@ryanio ryanio commented Aug 21, 2020

This PR:

  • Runs git stash before github-action-benchmark
  • Runs benchmarks on PRs
  • Updates the root package-lock.json

@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #840 into master will decrease coverage by 1.42%.
The diff coverage is n/a.

Impacted file tree graph

Flag Coverage Δ
#account 92.85% <ø> (ø)
#block 77.81% <ø> (?)
#blockchain 81.33% <ø> (+0.30%) ⬆️
#common 94.14% <ø> (+0.15%) ⬆️
#ethash 83.33% <ø> (+1.11%) ⬆️
#tx 94.16% <ø> (ø)
#vm 92.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@holgerd77
Copy link
Member

Not sure about this, feel free to update, but have given this a WIP label since benchmarks CI is showed as "failing".

@ryanio
Copy link
Contributor Author

ryanio commented Aug 24, 2020

@holgerd77 thanks! @evertonfraga's commit was the fix needed, just had to also apply it to vm-pr.yml (in addition to vm-build.yml) so this should be ready for review and merge now! I also rebased on master.

Sorry about the performance alerts, I'm hoping once this is running on master we'll get some normalization of the benchmark run times. If it's all over the place then I'll have to re-think the approach.

@evertonfraga evertonfraga self-requested a review August 24, 2020 19:29
Copy link
Contributor

@evertonfraga evertonfraga left a comment

Choose a reason for hiding this comment

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

I wish I could make the stash command simpler, but my tries led to syntax errors for the yaml parser. Maybe next iteration! :)

@evertonfraga evertonfraga merged commit d08ba7c into master Aug 25, 2020
@holgerd77 holgerd77 deleted the update-packagelock branch August 25, 2020 11:20
@holgerd77
Copy link
Member

@evertonfraga nice work, thanks! 🙂

@holgerd77 holgerd77 mentioned this pull request Dec 4, 2020
42 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants