-
Notifications
You must be signed in to change notification settings - Fork 239
add net 11 end to end tests #3543
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
base: main
Are you sure you want to change the base?
Conversation
31c9364 to
0ee3237
Compare
| var controllerFile = Path.Combine(TestProjectPath, "ProductsController.cs"); | ||
| Assert.True(File.Exists(controllerFile), $"Controller file not found: {controllerFile}"); | ||
|
|
||
| var controllerContent = File.ReadAllText(controllerFile); |
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 wonder if using async IO here would help the test suite to complete more quickly, as the test runner thread wouldn't be blocked here, and could do other work. I don't know enough about xUnit and collections to know if it'd be a factor here, but something to consider and should be an easy change.
Ditto other tests in this class.
| { | ||
| string runSkippableTests = Environment.GetEnvironmentVariable("SCAFFOLDING_RunSkippableTests"); | ||
| Skip.If(string.IsNullOrEmpty(runSkippableTests)); | ||
| Skip.If(true, "dotnet scaffold aspnet mvccontroller does not yet create files in expected locations. Tool uses dotnet new which creates files in project root with template naming."); |
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.
These first three tests don't do anything, as they always skip themselves. Is that intended?
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.
Actually, eight of the tests here are always skipped.
| string runSkippableTests = Environment.GetEnvironmentVariable("SCAFFOLDING_RunSkippableTests"); | ||
| Skip.If(string.IsNullOrEmpty(runSkippableTests)); |
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.
Can you explain this logic? These tests will only run if called from run-net11-e2e-tests.ps1, which sets this environment variable. What is this for? Does anything call that ps1 file, or will these tests never run during CI or IDE runs? Seems surprising.
| // Verify basic controller structure for .NET 11 | ||
| Assert.Contains("class ProductsController", controllerContent); | ||
| Assert.Contains("Controller", controllerContent); // Should inherit from Controller base class |
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.
Is there a reason we don't test the full content of the output?
On the one hand, checking the full content would find more regressions, but on the other hand it would make the tests more susceptible to breaks. Interested to understand the philosophy here.
| </Project> | ||
| "; | ||
|
|
||
| #region .NET 11 Project Templates |
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.
nit: consider raw strings (i.e. """) for these multiline source code strings. They're able to be indented in a way that doesn't break the indentation of the containing type's members.
e.g.
public const string Net11ProjectTxt = """
<Project ...>
""";
fixes #3536 beginning of end to end tests for
dotnet scaffoldfor net 11 scenarios.