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

Update tests to use raw strings #67059

Merged
merged 132 commits into from
Mar 1, 2023
Merged

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner February 26, 2023 01:55
@DoctorKrolic
Copy link
Contributor

Can you please revert UseDeconstructionTest changes, so we can merge #67052 to improve it instead?

Comment on lines +2174 to +2175
var source = """
int i = 0;
Copy link
Member

Choose a reason for hiding this comment

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

❓ Why is a mechanical factoring PR containing code changes? this should have preserved the spacing.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's the mechanical approach that removes starting/ending lines, and remove indentation. THat way we don't have tests that look like this:

var test = """

class C 
                        {
                               void Foo()
                               {
                               }
                        }
                   }

"""

But instead get the much more clearly desired:

var test = """
           class C 
           {
               void Foo()
               {
               }
           }
           """;

Comment on lines +2203 to +2206
TestState = {
Sources = { source },
OutputKind = OutputKind.ConsoleApplication,
},
Copy link
Member

Choose a reason for hiding this comment

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

❓ Why is a mechanical factoring PR containing code changes? This should not have changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

so changing the test here just changed diagnostic spans. That's pointless pain (since this test is not seeking to verify that you get a "Program using top-level statements must be an executable." error). So this was changed to remove the error, and just pass teh flag to teh compiler that ensures we test the thing we care about, without extraneous (position-sensitive) data in it.

@CyrusNajmabadi CyrusNajmabadi merged commit 1b40661 into dotnet:main Mar 1, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the rawStringTests branch March 1, 2023 01:03
@ghost ghost added this to the Next milestone Mar 1, 2023
@RikkiGibson RikkiGibson modified the milestones: Next, 17.6 P2 Mar 2, 2023
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.

5 participants