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

chore: add macos, linux in ci #513

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

get-me-power
Copy link
Contributor

@get-me-power get-me-power commented Feb 2, 2023

On Windows, NET Framework and .NET6 is tested on CI.

.NET6 testing is now done in CI on Linux and Mac.

@manfred-brands
Copy link
Member

Looks like the Microsoft.CodeAnalysis assembly tries to load SQLite and fails. But there is way to much output from normal succeeding tests.

@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<RootNamespace>NUnit.Analyzers.Tests</RootNamespace>
<TargetFrameworks>net6.0;net462</TargetFrameworks>
<TargetFramework>net6.0</TargetFramework>
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be done conditionally on OS. We still want to test .net framework on Windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with your suggestion.
Therefore, I changed only ci.yml like this.
552b72c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can test .NET and .NET Framework on windows, and .NET test on unix.

@get-me-power get-me-power marked this pull request as ready for review February 6, 2023 13:02
@get-me-power get-me-power marked this pull request as draft February 6, 2023 17:52
@manfred-brands
Copy link
Member

I'm currently traveling and only have access to my phone.
For coverage we should align with the PR currently underway for nunit.framework. Note that it is no longer necessary to include a NuGet package to output coverage files from dotnet test. I have to lookup what I did for my work.

Comment on lines +24 to +26
dotnet tool install --global dotnet-coverage
dotnet test --collect:"XPlat Code Coverage" -f net6.0 ./src/nunit.analyzers.tests/
dotnet-coverage merge -o merged.cobertura.xml -f cobertura -r coverage.cobertura.xml
Copy link
Member

Choose a reason for hiding this comment

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

There is no need for the coverlet nuget package. Dotnet can output cobertura natively..
You can call it like: dotnet test --collect:"Code Coverage;Format=Cobertura"

Comment on lines +8 to +11
<PackageReference Include="coverlet.collector" Version="3.2.0">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary anymore.

Comment on lines +1 to +21
coverage:
paths:
- merged.covertura.xml

codeToTestRatio:
code: # files to count as "Code"
- 'src/nunit.analyzers/**/*.cs'
test: # files to count as "Test"
- 'src/nunit.analyzers.tests/**/*.cs'

comment:
if: is_pull_request

report:
if: is_default_branch
datastores:
- artifact://${GITHUB_REPOSITORY}

diff:
datastores:
- artifact://${GITHUB_REPOSITORY}
Copy link
Member

Choose a reason for hiding this comment

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

This needs align with the nunit changes to use coverlet.

Comment on lines +33 to +35
- name: Post Coverage
uses: k1LoW/octocov-action@v0
if: runner.os == 'Linux'
Copy link
Member

Choose a reason for hiding this comment

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

This needs align with the nunit/nunit#4286 to use coverlet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants