-
Notifications
You must be signed in to change notification settings - Fork 43
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
test: Add flag to skip network tests #2495
test: Add flag to skip network tests #2495
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2495 +/- ##
========================================
Coverage 75.62% 75.63%
========================================
Files 291 291
Lines 28072 28072
========================================
+ Hits 21229 21230 +1
+ Misses 5487 5486 -1
Partials 1356 1356
Flags with carried forward coverage won't be shown. Click here to find out more. see 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
tests/integration/test_case.go
Outdated
// IsNetworkAction returns true if the given action involves the network subsystem. | ||
func IsNetworkAction(act any) bool { | ||
switch act.(type) { | ||
case ConfigureNode: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I don't think you need to check all of these? ConfigureNode
is required for all P2P tests IIRC. Expecting all future network test actions to consistently be added to this switch seems a little optimistic to me and I would rather avoid the need if we can.
suggestion: If keeping all the actions in the switch, I think collapsing them into a single case might be easier to read:
switch act.(type) {
case ConfigureNode,
ConnectPeers,
ConfigureReplicator,
etc...:
return true
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simplified it to just check for ConfigureNode
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
Relevant issue(s)
Resolves #2494
Description
This PR adds a test flag set via an environment variable to skip any tests that involve network actions.
Tasks
How has this been tested?
make test
Specify the platform(s) on which this was tested: