Skip to content
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

Modify assertLint test helper to run via pipeline only. #788

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

shawnhyam
Copy link
Contributor

Standalone lint visitors don't check for swift-format-ignore markers, so any test cases that use those markers will return different results between standalone vs full pipeline.

@allevato
Copy link
Member

allevato commented Aug 2, 2024

This makes me wonder if we even need the standalone-run mode anymore. Since we now double-check everything using the pipeline, it might be better to just have that be the single mode we care about, because the rule never runs in a standalone mode otherwise.

Could you make that change instead? Then test authors don't have to pick what mode they're interested in, which is cleaner.

@shawnhyam
Copy link
Contributor Author

Sure! From my point of view that's better -- I found it pretty confusing when I was getting different behavior between the two runs.

Standalone lint visitors don't check for swift-format-ignore markers,
so any test cases that use those markers will return different
results between standalone vs full pipeline. Since the app uses
the pipeline only, there is no value in running the rule standalone.
@shawnhyam shawnhyam changed the title Add an option to test lint rules on the full pipeline only. Modify assertLint test helper to run via pipeline only. Aug 2, 2024
@allevato allevato merged commit 87d5fb2 into swiftlang:main Aug 2, 2024
@shawnhyam shawnhyam deleted the swift-format-ignore branch August 6, 2024 12:48
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