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.

Sign up for GitHub

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

Jump to bottom

basicchain: use UnitTestNet default config for generation #3696

Merged
merged 2 commits into from
Dec 12, 2024
Merged

basicchain: use UnitTestNet default config for generation #3696

merged 2 commits into from
Dec 12, 2024

Conversation

AliceInHunterland
Copy link
Contributor

@AliceInHunterland AliceInHunterland commented Nov 22, 2024 •
edited
Loading

Close #3681

@AliceInHunterland
config: add UnitTestNet configuration to embed configs
8fe2ae8 
Tests should use embed config too.

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
@AliceInHunterland AliceInHunterland changed the title basicchain: use UnitTestNet default config basicchain: use UnitTestNet default config for generation Nov 22, 2024
AnnaShaleva
AnnaShaleva reviewed Nov 22, 2024
View reviewed changes
pkg/services/rpcsrv/server_test.go Outdated Show resolved Hide resolved
pkg/neotest/chain/chain.go Show resolved Hide resolved
pkg/core/blockchain_neotest_test.go Outdated Show resolved Hide resolved
@AliceInHunterland AliceInHunterland force-pushed the basic-chain-tests branch 2 times, most recently from f09b105 to a84023c Compare December 2, 2024 16:07
@AliceInHunterland AliceInHunterland marked this pull request as ready for review December 2, 2024 16:08
@codecov Codecov
Copy link

codecov bot commented Dec 2, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.90%. Comparing base (cdbc026) to head (e993c1b).
Report is 72 commits behind head on master.

Files with missing lines Patch % Lines
pkg/neotest/chain/chain.go 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3696      +/-   ##
==========================================
- Coverage   83.04%   82.90%   -0.14%     
==========================================
  Files         335      335              
  Lines       46708    46847     +139     
==========================================
+ Hits        38787    38837      +50     
- Misses       6328     6416      +88     
- Partials     1593     1594       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AnnaShaleva
AnnaShaleva reviewed Dec 11, 2024
View reviewed changes
pkg/services/rpcsrv/client_test.go Outdated Show resolved Hide resolved
config/protocol.unit_testnet.yml Outdated Show resolved Hide resolved
pkg/core/native/invocation_test.go Outdated Show resolved Hide resolved
pkg/core/blockchain_neotest_test.go Outdated Show resolved Hide resolved
pkg/core/native/invocation_test.go Outdated Show resolved Hide resolved
AnnaShaleva
AnnaShaleva reviewed Dec 11, 2024
View reviewed changes
Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

Let's fix 50ff7d3#r1879612949 and then LGTM.

@AliceInHunterland
basicchain: use UnitTestNet default config
e993c1b 
It's important to have the same chain configuration for all tests.
Otherwise, a mismatched hardfork configuration is used to dump/restore
the basic chain.

Close #3681

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AnnaShaleva
AnnaShaleva approved these changes Dec 12, 2024
View reviewed changes
Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

Good, I like this PR, it makes unittest runs unified.

@AnnaShaleva AnnaShaleva merged commit 6d1eea3 into master Dec 12, 2024
32 of 34 checks passed
@AnnaShaleva AnnaShaleva deleted the basic-chain-tests branch December 12, 2024 07:51
@AnnaShaleva AnnaShaleva mentioned this pull request Dec 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnnaShaleva AnnaShaleva AnnaShaleva approved these changes

@roman-khimov roman-khimov Awaiting requested review from roman-khimov roman-khimov is a code owner

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

Successfully merging this pull request may close these issues.

block20StateRootLE differ in basic chain generation and tests
2 participants
@AliceInHunterland @AnnaShaleva