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

feat: skip all the tests in short mode if they use Celestia testnode #155

Merged

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Feb 17, 2023

Overview

Skip all the tests when running in short mode if they use Celestia testnode because of the race conditions and not to have to skip each one separatly.

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@rach-id rach-id requested a review from rahulghangas February 17, 2023 10:16
@rach-id rach-id requested a review from evan-forbes as a code owner February 17, 2023 10:16
@rach-id rach-id self-assigned this Feb 17, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #155 (69752aa) into main (27f6375) will increase coverage by 5.76%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #155      +/-   ##
==========================================
+ Coverage   45.90%   51.66%   +5.76%     
==========================================
  Files          15       16       +1     
  Lines         952      991      +39     
==========================================
+ Hits          437      512      +75     
+ Misses        466      423      -43     
- Partials       49       56       +7     
Impacted Files Coverage Δ
cmd/qgb/helpers/parse.go 100.00% <0.00%> (ø)
p2p/querier.go 80.23% <0.00%> (+0.23%) ⬆️
p2p/dht.go 68.29% <0.00%> (+6.47%) ⬆️
orchestrator/orchestrator.go 15.76% <0.00%> (+15.76%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

I'm a tad confused on what exactly what we're skipping currently and why, mind elaborating? not necessarily for this PR, I think that makes sense, but I would like to know more before we proceed

are we using "-short" to mean not run the race detector? aren't most tests using the testnode? should we just run those tests without the race detector on?

@rach-id
Copy link
Member Author

rach-id commented Feb 19, 2023

In this repo, and others such as Celestia-app, we're running the tests with -race -short flags when running race.

This PR skips any test that has the -short flag if it uses the tesnode (because of the race conditions happening), by skipping them when creating the network instead of having to do it in every test.

Let me know if im not clear enough to reformulate

@rach-id rach-id merged commit 99ed6fe into celestiaorg:main Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants