Skip to content

Conversation

kevingranade
Copy link
Member

Summary

None

Purpose of change

When I tried to use reduce_tests.sh on #81077 I found it to be quite fragile and require a lot of babysitting for false positives (a positive in this case is the test fails in an expected way).

Also I found that the multidelta wrapper script was re-running the test suite redundantly under the assumption that it was doing test case reduction across multiple files and using a tool to semantically group c/c++ expressions, neither of which we were doing.

Describe the solution

Adds a stub showing how to test for a target output string instead of test success. This is the recommended way to use delta to avoid false negatives, I think it can handle false negatives from "this makes the build fail" or "this makes the whole thing crash", but false positives seem to make it wander off into un-fruitful directions.

Swaps from multidelta that makes things slower without giving any new features to singledelta.

Describe alternatives you've considered

Ideally this would be goverened by an argument or something instead of requiring manual editing, but I really do not have the brain juice for that right now.
Since this isn't running through the "flatten" process any more, I don't think adding and removing curly brackets around the test cases is necessary anymore, but it's also not breaking anything so whatever.
Possibly the "match against a string" workflow should be the default, but that requires parameter handling so bleh.

Testing

Ran it a bunch of times against #81077 and it was pretty reliable once I had it searching for the error string.

@github-actions github-actions bot added the Code: Tooling Tooling that is not part of the main game but is part of the repo. label Sep 19, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Auto-requesting reviews from non-collaborators: @jbytheway

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Sep 19, 2025
Copy link
Contributor

@ehughsbaird ehughsbaird left a comment

Choose a reason for hiding this comment

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

This is a good add! I've not had much of a problem with misc failures polluting the tool, but it has happened once or twice.

It's also good to see I'm not the only one who uses this :)

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 20, 2025
@jbytheway
Copy link
Contributor

FWIW, the reason I used multidelta was, as ehughsbaird says, that the other script has different names in different distros.

But I'm also glad to see this script is still seeing some use.

Copy link
Contributor

@ehughsbaird ehughsbaird left a comment

Choose a reason for hiding this comment

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

I think this is portable.

@kevingranade
Copy link
Member Author

Thanks! I've just been trying to keep focused on hordes so neglecting this a bit, but I'll go ahead and merge this in.

kevingranade and others added 2 commits September 27, 2025 10:47
Swaps from multidelta that makes things more difficult without giving any new features to singledelta

Adds a stub showing how to test for a target output string instead of test success
@GuardianDll GuardianDll merged commit bb89045 into master Sep 28, 2025
23 checks passed
@GuardianDll GuardianDll deleted the tweak-reduce-tests branch September 28, 2025 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants