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

feat: Add additional test annotations #8272

Merged
merged 138 commits into from
Mar 16, 2022
Merged

Conversation

brdji
Copy link
Contributor

@brdji brdji commented Mar 9, 2022

Proposed Changes

Add missing test annotations from the Bloxico lotus fork.

Checklist

Before you mark the PR ready for review, please make sure that:

  • All commits have a clear commit message.
  • The PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
    • area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
  • This PR has tests for new functionality or change in behaviour
  • If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
  • CI is green

@codecov
Copy link

codecov bot commented Mar 10, 2022

Codecov Report

Merging #8272 (eeae0c1) into master (4906962) will increase coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8272      +/-   ##
==========================================
+ Coverage   40.16%   40.29%   +0.12%     
==========================================
  Files         679      679              
  Lines       73992    73992              
==========================================
+ Hits        29719    29815      +96     
+ Misses      39093    38952     -141     
- Partials     5180     5225      +45     
Impacted Files Coverage Δ
blockstore/badger/blockstore_test_suite.go 96.50% <ø> (ø)
storage/wdpost_sched.go 75.49% <0.00%> (-11.77%) ⬇️
extern/sector-storage/worker_tracked.go 79.46% <0.00%> (-7.15%) ⬇️
markets/retrievaladapter/client_blockstore.go 62.50% <0.00%> (-6.25%) ⬇️
chain/sub/incoming.go 45.25% <0.00%> (-2.45%) ⬇️
chain/events/events_called.go 85.85% <0.00%> (-2.44%) ⬇️
extern/storage-sealing/currentdealinfo.go 71.17% <0.00%> (-1.81%) ⬇️
chain/sync_manager.go 73.29% <0.00%> (-1.56%) ⬇️
chain/stmgr/searchwait.go 68.58% <0.00%> (-1.29%) ⬇️
chain/gen/gen.go 68.19% <0.00%> (-1.23%) ⬇️
... and 34 more

@brdji brdji marked this pull request as ready for review March 10, 2022 13:48
@brdji brdji requested a review from a team as a code owner March 10, 2022 13:48
@jennijuju jennijuju requested a review from arajasek March 15, 2022 04:02
@BlocksOnAChain
Copy link
Contributor

BlocksOnAChain commented Mar 16, 2022

@brdji @TheDivic can you add more details about what needs to be merged, and can we see and link of the missing info on your dashboard?
https://lotus.systemtestmatrix.com/system/api?tab=overview

@TheDivic
Copy link
Contributor

@BlocksOnAChain As you can see most of the systems/subsystems are "gray" and have a bad score. That's because a large part of test annotations is not merged into lotus master, and the test crawler (run through CI) is extracting annotation from lotus master and can't find them. Thus, the STM shows those behaviors as untested.

@brdji
Copy link
Contributor Author

brdji commented Mar 16, 2022

As @TheDivic said, these annotations cannot currently be seen in the STM because the test crawler ignores these files - thus the tests are ignored and the behavior is marked as untested. This PR would annotate all of the remaining test files and tests, and they would show up correctly in the STM catalog, marking the corresponding behaviors as tested (depending on the test type - most of the tests in this PR are unit tests).
All of the required merge changes here are STM comments, and they have no effect on the behavior or performance of the existing codebase.

@magik6k magik6k merged commit 7945366 into filecoin-project:master Mar 16, 2022
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.

5 participants