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

test: re-enable disabled tests #7211

Merged
merged 1 commit into from
Aug 31, 2021
Merged

test: re-enable disabled tests #7211

merged 1 commit into from
Aug 31, 2021

Conversation

Stebalien
Copy link
Member

  1. Rewrite VRF test to actually test win counts, and enable it.
  2. Stop skipping some tests that now pass.

@Stebalien
test: re-enable disabled tests
8e52bf3 
1. Rewrite VRF test to actually test win counts, and enable it.
2. Stop skipping some tests that now pass.
@Stebalien Stebalien requested a review from a team as a code owner August 27, 2021 22:44
@codecov
Copy link

codecov bot commented Aug 27, 2021 •
edited
Loading

Codecov Report

Merging #7211 (8e52bf3) into master (714635c) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7211      +/-   ##
==========================================
+ Coverage   39.13%   39.15%   +0.01%     
==========================================
  Files         608      608              
  Lines       64602    64602              
==========================================
+ Hits        25283    25293      +10     
+ Misses      34930    34925       -5     
+ Partials     4389     4384       -5
Impacted Files Coverage Δ
chain/actors/builtin/paych/paych.go 16.88% <0.00%> (-5.20%) ⬇️
chain/stmgr/call.go 62.00% <0.00%> (-4.00%) ⬇️
extern/sector-storage/sched_worker.go 76.53% <0.00%> (-1.45%) ⬇️
chain/messagepool/selection.go 80.52% <0.00%> (-0.41%) ⬇️
chain/messagepool/messagepool.go 57.67% <0.00%> (+0.12%) ⬆️
storage/wdpost_changehandler.go 98.11% <0.00%> (+0.47%) ⬆️
chain/gen/gen.go 67.55% <0.00%> (+1.17%) ⬆️
extern/storage-sealing/precommit_batch.go 76.41% <0.00%> (+1.53%) ⬆️
itests/kit/blockminer.go 95.23% <0.00%> (+1.58%) ⬆️
miner/miner.go 54.96% <0.00%> (+2.31%) ⬆️
... and 3 more

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 714635c...8e52bf3. Read the comment docs.

@magik6k
Copy link
Contributor

magik6k commented Aug 28, 2021

Pretty sure TestWinCounts was written to just output to an external file for analyzing manually, cc @Kubuxu

ZenGround0
ZenGround0 reviewed Aug 30, 2021
View reviewed changes
chain/vectors/vectors_test.go
@@ -26,7 +26,6 @@ func LoadVector(t *testing.T, f string, out interface{}) {
}

func TestBlockHeaderVectors(t *testing.T) {
t.Skip("we need to regenerate for beacon")
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we done the vector regeneration described here?
Do we even run vector tests any more?

Copy link
Member Author

Choose a reason for hiding this comment

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

The tests pass.

chain/vectors/vectors_test.go
@@ -65,8 +64,6 @@ func TestMessageSigningVectors(t *testing.T) {
}

func TestUnsignedMessageVectors(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have the vectors been fixed?

Copy link
Member Author

Choose a reason for hiding this comment

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

The tests pass.

Copy link
Member Author

Choose a reason for hiding this comment

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

Although I'm not seeing any updates.

@Stebalien
Copy link
Member Author

Pretty sure TestWinCounts was written to just output to an external file for analyzing manually, cc @Kubuxu

They were but it should be automatic now.

@Stebalien Stebalien merged commit d113813 into master Aug 31, 2021
@Stebalien Stebalien deleted the test/audit-disabled-tests branch August 31, 2021 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZenGround0 ZenGround0 ZenGround0 approved these changes

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
@Stebalien @magik6k @ZenGround0