-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[FIX MAIN] Update the tests to reflect the new templates. #45287
Conversation
Resolves dotnet#45283 Ok so the template engine uses 'Verify' which I dont see elsewhere in the SDK -- What this does is store a txt snapshot of the last test result. It looks like Class, Record, Enum, Struct, etc may be new templates added in net 10 that need to be added to the test logic. dotnet#29655 They were actually added in 7.0.200. I dont know why but the last test result didnt have the templates added in 7.0.200 for net 10, maybe they were not updated and it got temporarily removed. I couldnt find that in the history, though, so I've added them now that theyre showing up. I dont think this test will pass because there is a whitespace output change as well. Im not sure if that applies to mac and linux or not. `DotnetNewListTests.BasicTest_WhenListCommandIsUsed.OSX.verified.txt` will have to be updated and `DotnetNewListTests.BasicTest_WhenListCommandIsUsed.Linux.verified.txt` will have to be with the new templates, if those fail, otherwise if the legacy tests also fail, they will have to be updated with the new templates and new whitespace.
@nagilson do you know which pr broke it? 10 hours ago ci was green then suddenly all prs started to fail. there is no "update from dotnet/templating" pr in between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd think it odd for these tests to suddenly start failing in this way. Maybe something else is going on? 🤔
I can buy into new templates popping up, that does happen, but the whitespace changes seem a bit strange. There were some PR failures, but they are spread across multiple branches |
Not yet. It's pretty strange behavior, honestly. |
I have pushed a new PR(#45300) to fix the template test failures. |
Looks like Jason fixed it in his PR. We should probably close this one and make sure to backport his PR to the other branches. |
Pull request was closed
Thank you ✨ |
Good point but I dont think this needs to be backported. This was more of a net 10 problem and its not impacting other branches. |
Resolves #45283
Ok so the template engine uses 'Verify' which I dont see elsewhere in the SDK -- What this does is store a txt snapshot of the last test result.
It looks like Class, Record, Enum, Struct, etc may be new templates added in net 10 that need to be added to the test logic. #29655
They were actually added in 7.0.200. I dont know why but the last test result didnt have the templates added in 7.0.200 for net 10, maybe they were not updated and it got temporarily removed. I couldnt find that in the history, though, so I've added them now that theyre showing up.
I dont think this test will pass because there is a whitespace output change as well. Im not sure if that applies to mac and linux or not.
DotnetNewListTests.BasicTest_WhenListCommandIsUsed.OSX.verified.txt
will have to be updated andDotnetNewListTests.BasicTest_WhenListCommandIsUsed.Linux.verified.txt
will have to be with the new templates, if those fail, otherwise if the legacy tests also fail, they will have to be updated with the new templates and new whitespace.You could just run the test in a vm and copy paste, worst case.