The Wayback Machine - https://web.archive.org/web/20210129195933/https://github.com/dotnet/corefxlab/pull/2893
Skip to content
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

Pass in -ci and -test to CI #2893

Merged
merged 5 commits into from Mar 26, 2020
Merged

Pass in -ci and -test to CI #2893

merged 5 commits into from Mar 26, 2020

Conversation

@pgovind
Copy link
Member

@pgovind pgovind commented Mar 25, 2020

I think this is all we needed. Putting this up to test what happens in a public build

corefxlab-base.yml Outdated Show resolved Hide resolved
corefxlab-base.yml Outdated Show resolved Hide resolved
@pgovind
Copy link
Member Author

@pgovind pgovind commented Mar 25, 2020

It looks like the CI systems are not able to locate xunit in their nuget cache. Not sure how to fix this?

@eerhardt
Copy link
Member

@eerhardt eerhardt commented Mar 25, 2020

How are you seeing the logs? I don't see the test logs being captured anywhere. what is the error?

@safern
Copy link
Member

@safern safern commented Mar 25, 2020

He reproed locally. We're looking at it offline. I need to disconnect now for a bit but I'll repro locally and figure it out later.

@eerhardt
Copy link
Member

@eerhardt eerhardt commented Mar 25, 2020

He reproed locally.

We should be capturing the logs as well, so that you don't need to rely on reproing it locally to figure out the issue.

@pgovind
Copy link
Member Author

@pgovind pgovind commented Mar 25, 2020

We should be capturing the logs as well, so that you don't need to rely on reproing it locally to figure out the issue.

We do actually. It's here: https://dev.azure.com/dnceng/public/_build/results?buildId=573155&view=artifacts&type=publishedArtifacts

That only reads "Path could not be found" but doesn't specify which exe is missing. I had to repro locally to figure out that it was xunit

@safern
Copy link
Member

@safern safern commented Mar 25, 2020

We also have this issue: #2708

I'll take care of it once we figure out this issue.

@@ -0,0 +1,22 @@
<RuleSet Name="Tests ruleset" Description="All Rules are disabled." ToolsVersion="15.0">

This comment has been minimized.

@pgovind

pgovind Mar 26, 2020
Author Member

This is something that xunit knows to interpret/use?

This comment has been minimized.

@safern

safern Mar 26, 2020
Member

No, this is something Roslyn understands.

The newest version of xunit brings in analyzers as well. This is the way to configure roslyn analyzers. Then you can set the severity to error, warning or none, on a per rule, per analyzer basis. So whenever we want to enable these rules, we will have to fix the errors we get by the analyzer.

This comment has been minimized.

@safern

safern Mar 26, 2020
Member

It was just a smaller change to do this and then we can enable the rules by chunks.

This comment has been minimized.

@pgovind

pgovind Mar 26, 2020
Author Member

I see. So when we want to turn on one of these rules, we change Action="None" -> say Action="Error"(or Warning)?

This comment has been minimized.

@safern

safern Mar 26, 2020
Member

Yeah, or if you don't list them in this file they will take whatever default Action the analyzer defines: http://xunit.net/xunit.analyzers/rules/

@safern
Copy link
Member

@safern safern commented Mar 26, 2020

Ok so I found the reason why tests were failing. The reason is because we now use arcade -test target to run our test projects, which invokes xunit.console.dll directly. However, we were overriding arcade version to 2.2.0, and from version 2.4.0 I believe the structure of the package changed, adding the TFM to the paths. So arcade was searching for the .dll with XunitPackageDir\netcoreapp2.0\xunit.console.dll.

https://github.com/dotnet/arcade/blob/d799f35d3ae963f3dc935a7dac1d4c791e85d8e6/src/Microsoft.DotNet.Arcade.Sdk/tools/XUnit/XUnit.targets#L55

So what I did is remove the version we were using and use arcade's version. However, that pulls in the xunit.analyzers which is goodness to write good tests. There were some build errors, so I added a Tests.ruleset to disable the errors, and we can then progressively fix them.

@safern safern force-pushed the pgovind:fixci2 branch from ab319de to 0094cfa Mar 26, 2020
@@ -41,8 +41,6 @@ public void CtorSpanOverByteArrayValidCasesWithPropertiesAndBasicOperationsCheck
Span<byte> span = new Span<byte>(array);
Assert.Equal(array.Length, span.Length);

Assert.NotSame(array, span.ToArray());

This comment has been minimized.

@safern

safern Mar 26, 2020
Member

It seems like in 3.0 this behavior changed and span.ToArray returns the same, or the test was wrong.

@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<TargetFrameworks>netcoreapp2.0;netcoreapp3.0</TargetFrameworks>

This comment has been minimized.

@safern

safern Mar 26, 2020
Member

We don't install any runtimes when bootstrapping, which we could by adding an entry to global.json, however, I don't think it makes sense to test against an older framework.

Copy link
Member

@eerhardt eerhardt left a comment

Looks fine to me. Nice work everyone.

@safern safern merged commit 7e4a48c into dotnet:master Mar 26, 2020
9 checks passed
9 checks passed
WIP Ready for review
Details
corefxlab-ci Build #20200325.8 succeeded
Details
corefxlab-ci (build Innerloop OSX x64_Debug) build Innerloop OSX x64_Debug succeeded
Details
corefxlab-ci (build Innerloop OSX x64_Release) build Innerloop OSX x64_Release succeeded
Details
corefxlab-ci (build Innerloop Ubuntu16.04 x64_Debug) build Innerloop Ubuntu16.04 x64_Debug succeeded
Details
corefxlab-ci (build Innerloop Ubuntu16.04 x64_Release) build Innerloop Ubuntu16.04 x64_Release succeeded
Details
corefxlab-ci (build Innerloop Windows_NT x64_Debug) build Innerloop Windows_NT x64_Debug succeeded
Details
corefxlab-ci (build Innerloop Windows_NT x64_Release) build Innerloop Windows_NT x64_Release succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants