Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

tests: add benchmark tests setup #1623

Merged
merged 3 commits into from
Jan 23, 2023
Merged

tests: add benchmark tests setup #1623

merged 3 commits into from
Jan 23, 2023

Conversation

Vvaradinov
Copy link
Contributor

@Vvaradinov Vvaradinov commented Jan 20, 2023 •
edited
Loading

Closes: #XXX

Description

This PR adds a new the initial setup for benchmark testing params getting and setting. Benchmark tests can be executed with the following command: benchmark=true go test -bench=. -count 15 setting benchmark=true in order to avoid conflicting with our Gingko test setup.

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@Vvaradinov Vvaradinov requested a review from a team as a code owner January 20, 2023 17:17
@Vvaradinov Vvaradinov requested review from 0a1c and facs95 and removed request for a team January 20, 2023 17:17
@github-actions github-actions bot added the Type: Tests issues and PR related to tests label Jan 20, 2023
MalteHerrmann
MalteHerrmann reviewed Jan 20, 2023
View reviewed changes
Copy link
Contributor

@MalteHerrmann MalteHerrmann left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work @Vvaradinov! Tested this an it works as expected.

My suggestion would be to set up corresponding Makefile command(s) as well to have a basis for added tests in the future: make test-benchmark.

Let me make a PR with a suggestion and we can discuss further.

EDIT: Just seen, that we already have make benchmark - we should work from there.

@facs95 facs95 marked this pull request as draft January 20, 2023 17:37
@github-actions github-actions bot removed the Type: Tests issues and PR related to tests label Jan 20, 2023
@Vvaradinov
Copy link
Contributor Author

@MalteHerrmann we would need to create a new CI pipeline for this as well and make sure all the raw records are stored somewhere so we can compare later.

@fedekunze
Copy link
Contributor

@Vvaradinov that's what the gobencher CI is for

@fedekunze fedekunze marked this pull request as ready for review January 21, 2023 12:05
@fedekunze
Copy link
Contributor

EDIT: Just seen, that we already have make benchmark - we should work from there.

@MalteHerrmann seems to be misconfigured as it also runs unit tests. Let's fix the benchmark config in a separate PR.

benchmark=true go test -bench=. -count 15 ./... is failing because of unrelated tests

@fedekunze fedekunze merged commit 9305788 into main Jan 23, 2023
@fedekunze fedekunze deleted the Vvaradinov/params-benchmark-tests branch January 23, 2023 10:00
@MalteHerrmann
Copy link
Contributor

https://github.com/Mergifyio backport release/v0.21.x

@mergify mergify bot mentioned this pull request Jan 23, 2023
Merged
@mergify
Copy link
Contributor

mergify bot commented Jan 23, 2023

backport release/v0.21.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jan 23, 2023
@Vvaradinov @mergify
tests(evm): add benchmark tests setup for params (#1623)
6fa1224 
* tests: add benchmark tests setup

* fix: localized benchmark tests to keeper

* update benchmark

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
(cherry picked from commit 9305788)
MalteHerrmann pushed a commit that referenced this pull request Jan 23, 2023
@mergify @Vvaradinov
tests(evm): add benchmark tests setup for params (#1623) (#1627)
00b0d44 
* tests: add benchmark tests setup

* fix: localized benchmark tests to keeper

* update benchmark

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
(cherry picked from commit 9305788)

Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
This was referenced Jan 24, 2023
Closed
Merged
fedekunze added a commit that referenced this pull request Jan 25, 2023
@MalteHerrmann @Vvaradinov
@mergify @mmsqe @fedekunze @yihuang @GAtom22
chore: sync main with v0.21.x branch (#1636)
7ee11cf 
* release: v0.21.0-rc1 changelog (#1606)

* changelog: added rc-1 changelog entry

* fix: typo fix rc1

* fix(rpc): bind default json-rpc listen address to localhost (backport #1613) (#1621)

* fix(rpc): bind default json-rpc listen address to localhost (#1613)

* update nix

* bind default json-rpc to 127.0.0.1

* add change doc

* Update CHANGELOG.md

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
(cherry picked from commit 3587015)

# Conflicts:
#	CHANGELOG.md

* address merge conflicts

Co-authored-by: mmsqe <tqd0800210105@gmail.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>

* fix(rpc): align fee history (backport #1611) (#1620)

* fix(rpc): align fee history (#1611)

* update nix

* add next fee in fee history

* fix test

* add change doc

* height + 1 for next fee

* cross check baseFeePerGas len

* Update tests/integration_tests/test_fee_history.py

Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>

* fix oldestBlock & align earliest input as eth

* update doc

* update nix

* isort test_fee_history.py

* fix test

* align rpc res as eth

* add cross check

* add baseFeePerGas len check

* add oldestBlock check

Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
(cherry picked from commit 47fdfd3)

# Conflicts:
#	CHANGELOG.md

* address merge conflicts

Co-authored-by: mmsqe <mavis@crypto.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* support customize db opener in `StartCmd` (backport #1615) (#1622)

* support customize db opener in `StartCmd` (#1615)

* support customize db opener in `StartCmd`

* Update CHANGELOG.md

* pass  to dbOpener, so we can control different options through cli flags

* add comments

(cherry picked from commit f1337aa)

# Conflicts:
#	CHANGELOG.md

* conflicts

Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* tests(evm): add benchmark tests setup for params (#1623) (#1627)

* tests: add benchmark tests setup

* fix: localized benchmark tests to keeper

* update benchmark

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
(cherry picked from commit 9305788)

Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>

* refactor(params): store all params under one key in evm module (backport #1617) (#1626)

* refactor(params): store all params under one key in evm module (#1617)

* refactor(params): store all params under one key in evm module

* refactor(params): add changelog entry

* refactor(params): update based on review comments. Remove params getter functions

* refactor(params): refactor params store key

* refactor(params): remove unnecessary store keys

* refactor(params): add paramSetPairs for backwards compatibility

* Update CHANGELOG.md

* refactor(params): add license to params_legacy file

* Apply suggestions from code review

* fix(evm): handle RC1 params during migration (#1624)

* fix(evm): handle RC1 params during migration

* migration

* fix: test case updated for RC1

* v5 migration

* tests

* tests pt2

* comment

* execute make proto-all

Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>

* Apply suggestions from code review

* rm dup vars

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>
(cherry picked from commit f07b14f)

# Conflicts:
#	CHANGELOG.md

* update changelog

Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>

* fix(evm): revert v4 migration changes (#1625) (#1628)

* fix(evm): revert v4 migration changes

* add check for chainID

* fix: renamed vars to avoid duplicate proto

* test

* fix: set default extraEIPs to nil

* fix: updated tests

Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
(cherry picked from commit 9bfa1ff)

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* minor improvement to changelog (#1629)

* adjust year to 2023 for v0.21.0-rc2 release (#1630)

* Update CHANGELOG.md

Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: mmsqe <tqd0800210105@gmail.com>
Co-authored-by: mmsqe <mavis@crypto.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MalteHerrmann MalteHerrmann MalteHerrmann left review comments

@fedekunze fedekunze fedekunze approved these changes

@0a1c 0a1c Awaiting requested review from 0a1c 0a1c is a code owner automatically assigned from evmos/core-engineering

@facs95 facs95 Awaiting requested review from facs95 facs95 is a code owner automatically assigned from evmos/core-engineering

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Vvaradinov @fedekunze @MalteHerrmann