@mthalman @MichaelSimons SBRP Validation tests require investigating the built packages. The reason the build is failing in this PR is that the job for running the tests does not execute a build (this was not an issue for GenerateScriptTests.cs as it was independent of the build job).

Due to this, some refactoring must be done. Given that the tests are run on Windows and Linux, I'm wondering if we would consider running 3 jobs rather than 4. This would mean changing the pipeline.

The current pipeline is:
Job 1: Run SB managed build and publish artifacts
Job 2: Run tests for Windows
Job 3: Run tests for Linux
Job 4: Run SB complete

My proposed pipeline is:
Job 1: Build, run tests, and publish artifacts Windows
Job 2: Build, run tests, and publish artifacts for Linux
Job 3: Run SB complete

The other alternative I can think of would be to publish the built packages from the current pipeline's job 1 in a tar.gz and then download and unpack them as a step in the current pipeline's jobs 2 and 3.

Job 1: Build, run tests, and publish artifacts Windows

Note that SBRPs can't be built on Windows today as building the targeting pack is currently only supported on Unix. It should be possible to change that though.

The proposed pipeline refactoring looks good and follows the recommended vertical build model. FWIW I'm not sure if merging the test projects together is an improvement. The existing test project doesn't require the SBRP packages to be generated while the new test does require that. I would probably keep them separated. That will allow you to keep the existing tests running on Windows.

Note that SBRPs can't be built on Windows today as building the targeting pack is currently only supported on Unix. It should be possible to change that though.

This is mentioned in more detail in GitHub Issue #3724.

The existing test project doesn't require the SBRP packages to be generated while the new test does require that. I would probably keep them separated.

I agree that even though the generate tests are independent of SBRP package generation, there is a case to be made for consolidating the test projects. As an example, the SmokeTests in the VMR are all contained within a single test project; this format promotes a more uniform code structure and reduces redundancy. Additionally, if there's a need to skip tests in the future, implementing tests as SkippableFacts/SkippableTheories would be a straightforward solution.

I agree that even though the generate tests are independent of SBRP package generation, there is a case to be made for consolidating the test projects. As an example, the SmokeTests in the VMR are all contained within a single test project; this format promotes a more uniform code structure and reduces redundancy. Additionally, if there's a need to skip tests in the future, implementing tests as SkippableFacts/SkippableTheories would be a straightforward solution.

We have hundreds of test projects in dotnet/runtime, because they test different components, i.e. System.Text.RegularExpressions.Tests.csproj tests the implementation of System.Text.RegularExpressions.csproj. The same is true for the generate-script tests and the tests that you are adding here. They are fundamentally different things.

this format promotes a more uniform code structure and reduces redundancy

I have to disagree on that. What enables a uniform code structure are analyzers and .editorconfig rules and ruleset files but definitely not a single test project.

Additionally, if there's a need to skip tests in the future, implementing tests as SkippableFacts/SkippableTheories would be a straightforward solution.

Straightforward sure, but you want to skip tests based on parameters like the machine configuration / the runtime or other traits. You don't want to express your build dependencies and order via xunit traits.

How would you keep the existing generate script tests enabled when merging the test projects? Disabling them on Windows, even temporarily isn't an improvement.

@ViktorHofer

I've had a few discussions with @MichaelSimons and @mthalman on the points you bring up. I can understand the reasons for splitting these tests into two different projects; the generate tests are testing the generation logic, and the SBRP tests are testing package "traits". Since they are testing two different components of the repo, I agree that there is reason behind splitting up the tests into two different projects. However, this is a small repo, much smaller than dotnet/runtime. I think there because of this, there is value to the tests sharing a singular test project in this repo given its size.

this format promotes a more uniform code structure and reduces redundancy

I recognize that this may not have come across as I had intended it to. I was not referring to the stylistic code structure of the tests, but rather the organization and grouping of data. My point was that by using a singular test project in this instance, we reduce the amount of repeated code, such as the RepoRoot variable and even the .proj files.

Straightforward sure, but you want to skip tests based on parameters like the machine configuration / the runtime or other traits. You don't want to express your build dependencies and order via xunit traits.

It is still possible to skip tests based on these factors. For example, it's possible to determine the OS version and skip based on that.

Ultimately, I acknowledge there are reasons to both sides. If there is a significant preference for multiple test projects over a singular test project, I am happy to make that change. As of right now, I am choosing to leave the tests in a singular project.

Ultimately, I acknowledge there are reasons to both sides. If there is a significant preference for multiple test projects over a singular test project, I am happy to make that change. As of right now, I am choosing to leave the tests in a singular project.

I'm still not seeing how you can make this change without disabling the existing tests on Windows. See my question above:

How would you keep the existing generate script tests enabled when merging the test projects? Disabling them on Windows, even temporarily isn't an improvement.

How would you keep the existing generate script tests enabled when merging the test projects? Disabling them on Windows, even temporarily isn't an improvement.

That logic is here. The test is a skippable fact, skipped if the OS is Windows.

I also have pipeline runs with the reworked pipeline. The first pipeline run includes the logic of SBRP validation tests as they exist in this PR (Generate and validation tests). The second pipeline run runs the tests as they currently exist in main (just the Generate tests).

• Pipeline run with SBRP validation tests: https://dev.azure.com/dnceng/internal/_build/results?buildId=2312957&view=results
• Pipeline run without SBRP validation tests: https://dev.azure.com/dnceng/internal/_build/results?buildId=2312971&view=results

I see. So you chose to skip all the tests that aren't supported on Windows. I'm fine with that. My biggest concern was that you would need to disable the existing generate script tests on Windows.

Ultimately, I acknowledge there are reasons to both sides. If there is a significant preference for multiple test projects over a singular test project, I am happy to make that change

That's the reason: > Since they are testing two different components of the repo

That said, I'm fine with your proposal given in its current form. I'm just not convinced that this is a good long term strategy. As an example, if I would want to add tests for this msbuild task, I would not put them into this single test project as such tests would depend on msbuild and NuGet APIs. This is a different component.

However, this is a small repo, much smaller than dotnet/runtime. I think there because of this, there is value to the tests sharing a singular test project in this repo given its size.

1. There is precedence, take a look at Arcade which is similar in size to this repo: https://github.com/dotnet/arcade/tree/main/src. Tests exist per component.
2. Repositories grow organically and I would be surprised if that won't be true for SBRP.

@ViktorHofer Thanks for your input; you bring up valid arguments for why there should be two test projects. However, after discussing this in more depth with @MichaelSimons, we've decided to stick with a single test project implementation as it exists in this PR given the current state of this repo.

This will fail until the arcade sdk from dotnet/installer#17985 is merged in due to the change of the inner clone path. Until then, here is a success run with these changes: https://dev.azure.com/dnceng/internal/_build/results?buildId=2336530&view=results