Skip to content

Improvements for integration tests#24

Merged
brianjohnson5972 merged 3 commits into
Wire-Network:masterfrom
brianjohnson5972:nonparallelizable-tests-fix
Jun 28, 2025
Merged

Improvements for integration tests#24
brianjohnson5972 merged 3 commits into
Wire-Network:masterfrom
brianjohnson5972:nonparallelizable-tests-fix

Conversation

@brianjohnson5972

@brianjohnson5972 brianjohnson5972 commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

Changed label for tests that we expect to fail and marked some longer tests with medium_running_tests to reduce the runtime for this group

Other

Added nonparallelizable_failing_tests and long_running_failing_tests labels to allow avoiding running tests that have not been fixed to work with changes made to the Wire network resources allocations.

@brianjohnson5972
brianjohnson5972 changed the base branch from sysio-update to master June 19, 2025 20:23
@valthon

valthon commented Jun 22, 2025

Copy link
Copy Markdown
Contributor

I appreciate switching the label for long running tests that are (currently) failing, as that's consistent with the other changes we've made to narrow down the "must pass" suite. However, I'm not in favor of adding medium_running_tests as it really feels redundant with the long_running_tests label. Too much nuance here is just asking for missed tests and confused future developers. Either it's a long-running test or it's fast enough to run in our normal set.

@bearcubsvet

Copy link
Copy Markdown
Collaborator

I appreciate switching the label for long running tests that are (currently) failing, as that's consistent with the other changes we've made to narrow down the "must pass" suite. However, I'm not in favor of adding medium_running_tests as it really feels redundant with the long_running_tests label. Too much nuance here is just asking for missed tests and confused future developers. Either it's a long-running test or it's fast enough to run in our normal set.

Do you have any flag name in mind @valthon ?

@brianjohnson5972

brianjohnson5972 commented Jun 23, 2025 via email

Copy link
Copy Markdown
Contributor Author

@valthon

valthon commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

yeah, i think we just stick with one flag. if we split "medium" and "long" then what's to stop us from having "sort of long," "really long," "no more than 2 minutes," "less than 18 seconds," etc. It's better practice to stick to a binary for that sort of differentiation. The basic assumption is that developers will usually only run the fastest option unless they need full coverage in which case the longest option needs to be run. Any "middle ground" turns out to have extremely minimal benefits in exchange for unnecessary complexity. In scenarios where you're not just running the fastest or the most coverage you're usually only needing to run one or a small number of explicitly-relevant tests -- extremely rare that a full "long but not really long" suite adds value

@brianjohnson5972

brianjohnson5972 commented Jun 23, 2025 via email

Copy link
Copy Markdown
Contributor Author

@bearcubsvet

Copy link
Copy Markdown
Collaborator
Screenshot 2025-06-23 at 9 46 40 AM

@brianjohnson5972

@dtaghavi

Copy link
Copy Markdown
Contributor

yeah, i think we just stick with one flag. if we split "medium" and "long" then what's to stop us from having "sort of long," "really long," "no more than 2 minutes," "less than 18 seconds," etc. It's better practice to stick to a binary for that sort of differentiation. The basic assumption is that developers will usually only run the fastest option unless they need full coverage in which case the longest option needs to be run. Any "middle ground" turns out to have extremely minimal benefits in exchange for unnecessary complexity. In scenarios where you're not just running the fastest or the most coverage you're usually only needing to run one or a small number of explicitly-relevant tests -- extremely rare that a full "long but not really long" suite adds value

I agree with this, that's probably what I'd do run the fastest and once I am "finished" run the full suite. Don't want it to get to convoluted.

@dtaghavi dtaghavi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just remove the medium-running from README to match your latest commit, otherwise looks good to me.

@brianjohnson5972
brianjohnson5972 merged commit fe285d8 into Wire-Network:master Jun 28, 2025
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.

4 participants