/azp run

/azp run

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base(c5e3c40) to head(1a82a49).
Report is 4 commits behind head on main.

@@            Coverage Diff             @@##             main   #3251       +/-   ##==========================================- Coverage   72.98%       0   -72.99%==========================================  Files         299       0      -299       Lines       57215       0    -57215     ==========================================- Hits        41760       0    -41760+ Misses      15455       0    -15455

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

/azp run

/azp run

@edwardneal ok, so looking at these failures, I thiiiink I understand why these failures are happening. Sorta. The MetricsTest file is only pulled into the test project if it's on test set 3 (or no test set), that's why only some of the test jobs are failing (specifically the ones that end with 3). That test file has references to the event source class, which is internal. Since we don't have internalsvisibleto the test projects (yet ...@mdaigle did you give up on that?), I don't think we can access that class from the test project.

The part I don't get tho, if I build it in the IDE, it doesn't complain at all. But that build specifies no test set, which makes me wonder if something including a file or property that makes it all work. Nothing stood out at me, but I didn't go through it with a fine toothed comb.

@benrr101 I'm going to revert the change I made to allow internalsVisibleTo because, like you saw, it works in the IDE, but not in MSBuild, which is confusing. MSBuild compiles using the reference assemblies which don't currently contain any of the internal types. Visual studio (and I'm assuming Rider, too) compiles using the implementation assemblies, which do contain the internal types.

#3268

@edwardneal Update - apparently something is really messed up with how we have our projects set up.... As per@mdaigle, the ref projects are being used for the internalsvisibleto when building from command line, but the implementation projects are being used when building from IDE. Thus, we see the failures of your unit tests when we build the project during CI. I don't think we have a good plan for fixing this anytime soon (it's sort of predicated on a lot of other things being fixed). But in the meantime, Malcolm is going to back out the internalsvisibleto change so people don't get the wrong idea that they can use internal classes in test projects safely.

To address this ... I don't know what's best. I'd like to keep the tests in the codebase so we can use them someday, but maybe just comment them out? (it grosses me out to even type that)

Good catch, thanks - I'd not yet seen the test results. It's worth keeping in mind that this test originally used reflection; I'll swap it back to doing so, and we can avoid the problem completely.

I've adjusted MetricsTest to use reflection again - could I have another CI run please?

/azp run

@edwardneal please don't mind that I just went ahead and fixed the merge conflict for you :)

/azp run