I haven't had a chance to look at this in detail yet, but one thing I'd suggest is that instead of having a big cross-targeting project that tests targeting everything, just test each target individually. That will make it simpler to see what's wrong and fix it if something is failing.

@dsplaisted Ok, I've added one test per TFM in a new test class GivenThatWeWantToBuildALibraryWithTfm. They still all fail in the test harness but build locally using the environment variable path override. There has to be something simple in the test methods/harness themselves that's causing this. Would appreciate the help to see why they're not building.

Okay update -- got the tests working and passing. There were a few bug fixes and one really important thing -- only works with FullMSBuild.

So the tests must be run with .\build.ps1 -FullMSBuild to pass. Not sure how the tests should detect if they're on CoreCLR and not run/fail.

I still need to review all the details, but two main thoughts from cursory review:

1. For the individual TFM tests, you can have just one test asset and use a [Theory] with inlinedata for each TFM and use WithProjectChanges to insert the TFM for each. There are several like that already so search for the pattern an emulate it.

2. For the portable matching, the if contains("net45") but not contains("net451") patterns seems quite error prone. Can you normalize the thing you're matching against to always have leading and trailing separator and no whitespace or redundant separators, then simply say if contains("[separator]net45[separator]")?

I think we'll need to detect cases where language targets as missing and go with inconclusive/skipped result. It's not sustainable to have to have tons of VS workloads and VS 2015 installed for tests to pass. We should make arrangements to run in that environment often enough as an integration test, but we can't have unit tests depending on it always.

For the portable matching, the if contains("net45") but not contains("net451") patterns seems quite error prone. Can you normalize the thing you're matching against to always have leading and trailing separator and no whitespace or redundant separators, then simply say if contains("[separator]net45[separator]")?

Hmm...maybe? So like put a + in the beginning and end of the strings. Yes, I can try that. I agree that the pattern was annoying.

I think we'll need to detect cases where language targets as missing and go with inconclusive/skipped result. It's not sustainable to have to have tons of VS workloads and VS 2015 installed for tests to pass. We should make arrangements to run in that environment often enough as an integration test, but we can't have unit tests depending on it always.

How do you think the rest of the world feels about not having a true "xcopyable" build-server definition ;)

That said, I agree and am happy to update the tests to accommodate however that needs to be done.

How do you think the rest of the world feels about not having a true "xcopyable" build-server definition ;)

Fair. Not saying this is a situation to be proud of, just what we need to do to test given today's reality. ;)

@nguerrera I simplified the contains searches by including the separator char on either end to be more explicit and avoid partial matches.

As for using a theory for those tests, the one issue there is that the output isn't necessarily the same across all TFM's. Some include a .dll.mdb file (iOS, etc), some include .pri files. A good indicator the build used the correct targets is to verify those particular files are present for those TFM's as they wouldn't get created otherwise.

Is there any harm in having an explicit test for each TFM rather than use a theory?

Okay, tests are working as expected. The CI server needs to have VS 2015 with the Win8, SL5 and WP workloads installed to complete. The VS 2017 install needs the Xamarin and UWP workloads installed. Once those are on the FullFramework CI box, the tests will pass.

Even if we use distinct facts, we should avoid having dozens of nearly identical test projects to maintain. I think WithSourceChanges(... /* set TFM */) is still a good idea.

Note that as of right now, each test is copying the entire LibraryWithTfm folder leading to O(N^2) cspojs in bin\Debug\Tests where N is number of TFMs being tested. At a minimum, we need to just copy the test project that is used, but I strongly want to dedup them in source control too. It should be easy to add new TFMs moving forward without having to copy a bunch of files. You can also group tests which have the same expectations in to theories.

As I said earlier, we need build -FullMSBuild to pass locally without the Dev15 and Xamarin requirements. Can the tests check if the language targets exist and if not expect the missing language targets failure? We can then start a separate conversation about how to maximimize the coverage, but I'm not keen on a hard ordering of a huge dependency increase for CI to this PR.

@nguerrera I can dedup the tests, but checking for the targets existence is hard since MSBuildExtensionsPath isn't set today in the current way the tests are built. The tests are built and run on CoreCLR and then exec MSBuild to do the build of the test project itself based on the -FullMSBuild flag.

How do you suggest we get that variable from?

How do you suggest we get that variable (MSBuildExtensionsPath) from?

For test purposes, we can get it relative from the DOTNET_SDK_TEST_MSBUILD_PATH environment variable that locates msbuild.exe.