Skip to content

Improvements to test case reduction using swift-parser-test #633

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

Merged
merged 2 commits into from
Aug 24, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 24, 2022

  • Increase reduce timeout in swift-stress-tester to 2 seconds
    • I’ve had a few cases where parsing didn’t finish in 1 second but 2 were sufficient (in debug build of swift-syntax-test). Increase the timeout so that reduction works properly
  • Stop reduce subcommand if it’s not making progress
    • If the chunk sizes are really tiny compared to the reduced source file, it looks like we aren't making any progress reducing. Just abort the reduction in that case.

I’ve had a few cases where parsing didn’t finish in 1 second but 2 were sufficient (in debug build of swift-syntax-test). Increase the timeout so that reduction works properly
If the chunk sizes are really tiny compared to the reduced source file, it looks like we aren't making any progress reducing. Just abort the reduction in that case.
@ahoppen ahoppen requested a review from bnbarham August 24, 2022 07:20
@ahoppen
Copy link
Member Author

ahoppen commented Aug 24, 2022

@swift-ci Please test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Nit on:

Increase reduce timeout in swift-stress-tester to 2 seconds
Ie. both "increase" and "reduce" :)

What made you add the chunk size abort? Were you noticing the reduction taking a long time on large files but not actually getting anywhere?

@ahoppen
Copy link
Member Author

ahoppen commented Aug 24, 2022

Nit on:

Increase reduce timeout in swift-stress-tester to 2 seconds
Ie. both "increase" and "reduce" :)

This was meant as Increase “reduce timeout”, i.e. increase the timeout for the reduce subcommand. I’ll just merge it as is though, not sure if updating the commit message is worth having to run CI again.

What made you add the chunk size abort? Were you noticing the reduction taking a long time on large files but not actually getting anywhere?

This was because of files that failed to parse in the 2 second timeout. If we reduced them too much, they started parsing in under one second.

@ahoppen ahoppen merged commit 17b81be into swiftlang:main Aug 24, 2022
@ahoppen ahoppen deleted the pr/swift-syntax-test-changes branch August 24, 2022 10:33
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.

2 participants