Skip to content

Conversation

@MiYanni
Copy link
Member

@MiYanni MiYanni commented Apr 9, 2025

Summary

It was driving me a little crazy at how confusing the test folder structure was. There are different generations of tests and test projects that exist in this folder. It seems most just migrated things here but never did the due diligence to make things make logical sense. Instead, they just kinda jammed a bunch of <Compile> nodes into the dotnet.Tests project and called it a day.

What did I do? I looked at generally where things should go based on the structure that I recently revamped in the dotnet.csproj. Ironically, you can get a decent idea of the history of the test projects by looking at the AssemblyInfo.cs for the dotnet project. Then, I tried to figure out if anything else should be included. This PR, IN NO WAY, is the ideal structure, naming, or amount of TLC that is actually needed for this stuff. But it is a first blush to get things a bit more organized.

Changes

  • Created a folder layout in dotnet.Tests project that mimics the folder hierarchy of the dotnet project
    • Tests for commands exist now in the CommandTests folder
    • Tests for general CLI concepts exist now in their respective locations as they do in the dotnet project, such as CommandFactoryTests, ShellShimTests, etc.
  • Moved all the code from the random dotnet-XXXXX folders that only contained .CS files into the dotnet.Tests project
  • Moved the tests from dotnet-msbuild.Tests and dotnet-sln.Tests into dotnet.Tests project
  • Renamed the folder of dotnet-new.Tests for the dotnet-new.IntegrationTests project to dotnet-new.IntegrationTests
  • Renamed the folder of dotnet-format.Tests/tests for the dotnet-format.UnitTests project to dotnet-format.UnitTests
  • Deleted Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/.editorconfig since it was unused
  • Cleaned up AssemblyInfo.cs for the dotnet project since it was sharing internals with projects that no longer exist
  • No code changes, only project/build changes that reflect the files being moved around

Other Information/Not-Changes

  • There are Configurer tests that I put into a ConfigurerTests folder in the dotnet.Tests project, since they already existed in this project. However, I think that they somehow relate to the assembly Microsoft.DotNet.Configurer, but this has no separate test project. I think the right thing to do would be to make a separate test project for this assembly. But I didn't do that in this PR.
  • The Microsoft.DotNet.PackageInstall.Tests project could actually be integrated with dotnet.Tests since it tests the ToolPackage logic from dotnet.csproj. However, this project has a bunch of custom configuration with in the .CSPROJ and wasn't worth the effort to combine with dotnet.Tests. Thus, remains a separate project.
  • Test projects that remained independent of dotnet.Tests after investigation
    • containerize.UnitTests tests containerize which is a separate tool used by VS
    • dotnet-format.UnitTests is for dotnet-format which is a separate dotnet tool
    • dotnet-MsiInstallation.Tests is for the VM-based installation tests
    • dotnet-new.IntegrationTests is the separate dotnet new set of tests based on the Templating repo integration
    • dotnet-watch.Tests is for dotnet-watch which is a separate dotnet tool
  • Test projects that look related, but are actually MSBuild test projects (not dotnet CLI related)
    • Microsoft.NET.Build.Tests
    • Microsoft.NET.Clean.Tests
    • Microsoft.NET.Pack.Tests
    • Microsoft.NET.Publish.Tests
    • Microsoft.NET.Rebuild.Tests
    • Microsoft.NET.Restore.Tests
  • The file names, class names, and namespaces of the tests should be changed, but I did not change them for this PR. That takes a lot of work and I don't think the effort is worth it for the tests at this time.

Missing Tests????

I did not find tests for the following commands:

  • Package/Search
  • Workload/Config
  • Workload/Elevate
  • Workload/History
  • Hidden/InternalReportInstallSuccess
  • Hidden/Parse

MiYanni added 5 commits April 8, 2025 15:52
…since it was unused. Moved dotnet-msbuild.Tests and dotnet-sln.Tests to the dotnet.Tests project. Moved crossgen.Tests, dotnet-back-compat.Tests, and dotnet-completions.Tests to appropriate folders within dotnet.Tests. Renamed dotnet-new.Tests folder to dotnet-new.IntegrationTests. Moving dotnet-format.UnitTests up, and will rename folder in next commit.
… to mention that I previously moved the dotnet-sln.Tests to dotnet.Tests.
….UnitTests.csproj build. Renamed misnamed file.
@MiYanni MiYanni requested review from a team and Copilot April 9, 2025 01:54
@MiYanni MiYanni requested review from a team, arunchndr and tmat as code owners April 9, 2025 01:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 983 out of 990 changed files in this pull request and generated no comments.

Files not reviewed (7)
  • .editorconfig: Language not supported
  • CODEOWNERS: Language not supported
  • TemplateEngine.slnf: Language not supported
  • sdk.sln: Language not supported
  • src/BuiltInTools/dotnet-format.slnf: Language not supported
  • src/Tasks/Microsoft.NET.slnf: Language not supported
  • test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/.editorconfig: Language not supported

@ghost ghost added the untriaged Request triage from a team member label Apr 9, 2025
Copy link
Contributor

@edvilme edvilme left a comment

Choose a reason for hiding this comment

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

I looked at the tree in the file view to see the resulting structure (it is very hard to review large amounts of renames in Github. even more if half the path gets hidden away and you have to hover over every file) and it makes sense to me.

I like how dotnet.Tests.CommandTests matches the structure of the commands folder.

For format, msbuild, and new, would using dashes (-) not cause the same namespace issues dotnet-sln.Tests had for your previous PR Just read the part about renaming on the PR description :)


#nullable disable

using Microsoft.DotNet.Cli.Commands.Package.List;
Copy link
Contributor

@edvilme edvilme Apr 10, 2025

Choose a reason for hiding this comment

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

I have no context whatsoever about this file other than what we talked about offline, but is there a reason this isn't required for accessing ListPackageCommand??

Copy link
Member Author

@MiYanni MiYanni Apr 10, 2025

Choose a reason for hiding this comment

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

That isn't what you think it is. ListPackageCommand is this. What that is is a basically a way for the tests to execute dotnet list package. Meaning, this class has nothing to do with the actual class, which is here, but it will run the command, which eventually calls the code in that class. So, it doesn't directly access the code programmatically, but it does access it from running the CLI. All this is quite old.

To further clarify on the CLI, dotnet list package and dotnet package list do call that same code, @Forgind 's changes just has a shim for the former that calls the underlying code of the latter. So, unless we actually remove that shim, all this still is testing the same bits.

Lastly, this isn't a traditional "unit test" since it doesn't access the product code directly. It is actually an end-to-end test on a single feature since it executes the entire product to test this functionality.

@MiYanni
Copy link
Member Author

MiYanni commented Apr 10, 2025

image

😑

@MiYanni MiYanni merged commit d2e279d into dotnet:main Apr 10, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-CLI untriaged Request triage from a team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants