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

Use VSBuild instead of dotnet build #1177

Merged
merged 8 commits into from
Feb 17, 2022
Merged

Conversation

neilboyd
Copy link
Contributor

@neilboyd neilboyd commented Feb 8, 2022

The CI build started failing when Windows 2022 became the windows-latest build agent.
This is because of an issue that was causing Kurdish locale tests to fail.
This PR uses the VSBuild task instead of dotnet build in order to mitigate the issue.


Here is a checklist you should tick through before submitting a pull request:

  • Implementation is clean
  • Code adheres to the existing coding standards; e.g. no curlies for one-line blocks, no redundant empty lines between methods or code blocks, spaces rather than tabs, etc.
  • No Code Analysis warnings
  • There is proper unit test coverage
  • If the code is copied from StackOverflow (or a blog or OSS) full disclosure is included. That includes required license files and/or file headers explaining where the code came from with proper attribution
  • There are very few or no comments (because comments shouldn't be needed if you write clean code)
  • Xml documentation is added/updated for the addition/change
  • Your PR is (re)based on top of the latest commits from the main branch (more info below)
  • Link to the issue(s) you're fixing from your PR description. Use fixes #<the issue number>
  • Readme is updated if you change an existing feature or add a new one
  • Run either build.cmd or build.ps1 and ensure there are no test failures

@neilboyd neilboyd marked this pull request as ready for review February 8, 2022 08:14
@clairernovotny
Copy link
Member

Can you please open an issue on the pipelines repo so this can be tracked and fixed?

@neilboyd
Copy link
Contributor Author

neilboyd commented Feb 8, 2022

Can you please open an issue on the pipelines repo so this can be tracked and fixed?

@clairernovotny Which repo is that?

@clairernovotny
Copy link
Member

@neilboyd neilboyd changed the title use windows 2019 build agent Fix Kurdish local in VS 2022 Feb 9, 2022
@neilboyd
Copy link
Contributor Author

neilboyd commented Feb 9, 2022

The problem is not with the build agent. I've narrowed it down a bit and updated the description.
Maybe someone who understands Kurdish could add some insight? @mhmd-azeez?

@neilboyd neilboyd marked this pull request as draft February 9, 2022 07:10
@neilboyd neilboyd changed the title Fix Kurdish local in VS 2022 Fix Kurdish locale in VS 2022 Feb 9, 2022
@clairernovotny
Copy link
Member

clairernovotny commented Feb 9, 2022

I'm definitely not skipping tests. There may be a bug somewhere in the cli though that should be filed: https://github.com/microsoft/vstest

@mhmd-azeez
Copy link
Contributor

Hello @neilboyd, I am not entirely sure why this is happening. I added a test case to ResourcesTests:

[Fact]
[UseCulture("ku")]
public void CanGetCultureSpecificTranslationsWithImplicitCultureKurdish()
{
    var format = Resources.GetResource("DateHumanize_MultipleYearsAgo");
    Assert.Equal("{0} ساڵ لەمەوبەر", format);
}

When I build using CLI:

Mo in D:\code\oss\Humanizer\src on main ● λ dotnet test .\Humanizer.sln --filter CanGetCultureSpecificTranslationsWithImplicitCultureKurdish
  Determining projects to restore...
  All projects are up-to-date for restore.
  Humanizer -> D:\code\oss\Humanizer\src\Humanizer\bin\Debug\netstandard1.0\Humanizer.dll
  Humanizer -> D:\code\oss\Humanizer\src\Humanizer\bin\Debug\netstandard2.0\Humanizer.dll
  Humanizer -> D:\code\oss\Humanizer\src\Humanizer\bin\Debug\net6.0\Humanizer.dll
  Humanizer.Tests -> D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net48\Humanizer.Tests.dll
  Humanizer.Tests -> D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\netcoreapp3.1\Humanizer.Tests.dll
  Humanizer.Tests -> D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net6.0\Humanizer.Tests.dll
  Humanizer.Tests -> D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net5.0\Humanizer.Tests.dll
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net6.0\Humanizer.Tests.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:04.07]     CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [FAIL]
  Failed CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [3 ms]
  Error Message:
   Assert.Equal() Failure
              ↓ (pos 4)
Expected: {0} ساڵ لەمەوبەر
Actual:   {0} years ago
              ↑ (pos 4)
  Stack Trace:
     at Humanizer.Tests.Localisation.ResourcesTests.CanGetCultureSpecificTranslationsWithImplicitCultureKurdish() in D:\code\oss\Humanizer\src\Humanizer.Tests.Shared\Localisation\ResourcesTests.cs:line 29

Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: < 1 ms - Humanizer.Tests.dll (net6.0)
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net48\Humanizer.Tests.dll (.NETFramework,Version=v4.8)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:08.02]     CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [FAIL]
  Failed CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [103 ms]
  Error Message:
   Assert.Equal() Failure
              ↓ (pos 4)
Expected: {0} ساڵ لەمەوبەر
Actual:   {0} years ago
              ↑ (pos 4)
  Stack Trace:
     at Humanizer.Tests.Localisation.ResourcesTests.CanGetCultureSpecificTranslationsWithImplicitCultureKurdish() in D:\code\oss\Humanizer\src\Humanizer.Tests.Shared\Localisation\ResourcesTests.cs:line 29

Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration:  - Humanizer.Tests.dll (net48)
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net5.0\Humanizer.Tests.dll (.NETCoreApp,Version=v5.0)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:04.20]     CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [FAIL]
  Failed CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [5 ms]
  Error Message:
   Assert.Equal() Failure
              ↓ (pos 4)
Expected: {0} ساڵ لەمەوبەر
Actual:   {0} years ago
              ↑ (pos 4)
  Stack Trace:
     at Humanizer.Tests.Localisation.ResourcesTests.CanGetCultureSpecificTranslationsWithImplicitCultureKurdish() in D:\code\oss\Humanizer\src\Humanizer.Tests.Shared\Localisation\ResourcesTests.cs:line 29

Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: < 1 ms - Humanizer.Tests.dll (net5.0)
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\netcoreapp3.1\Humanizer.Tests.dll (.NETCoreApp,Version=v3.1)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:04.09]     CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [FAIL]
  Failed CanGetCultureSpecificTranslationsWithImplicitCultureKurdish [4 ms]
  Error Message:
   Assert.Equal() Failure
              ↓ (pos 4)
Expected: {0} ساڵ لەمەوبەر
Actual:   {0} years ago
              ↑ (pos 4)
  Stack Trace:
     at Humanizer.Tests.Localisation.ResourcesTests.CanGetCultureSpecificTranslationsWithImplicitCultureKurdish() in D:\code\oss\Humanizer\src\Humanizer.Tests.Shared\Localisation\ResourcesTests.cs:line 29

Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: < 1 ms - Humanizer.Tests.dll (netcoreapp3.1)

Meanwhile when I build using VS 2022:

Mo in D:\code\oss\Humanizer\src on main ● λ dotnet test .\Humanizer.sln --filter CanGetCultureSpecificTranslationsWithImplicitCultureKurdish --no-build
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net6.0\Humanizer.Tests.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: < 1 ms - Humanizer.Tests.dll (net6.0)
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net48\Humanizer.Tests.dll (.NETFramework,Version=v4.8)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration:  - Humanizer.Tests.dll (net48)
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\net5.0\Humanizer.Tests.dll (.NETCoreApp,Version=v5.0)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: < 1 ms - Humanizer.Tests.dll (net5.0)
Test run for D:\code\oss\Humanizer\src\Humanizer.Tests\bin\Debug\netcoreapp3.1\Humanizer.Tests.dll (.NETCoreApp,Version=v3.1)
Microsoft (R) Test Execution Command Line Tool Version 17.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: < 1 ms - Humanizer.Tests.dll (netcoreapp3.1)

I am not sure why, but for some reason, when the solution is built using the CLI, the tests can't find Kurdish resources. One thing I can think of is on Windows Central Kurdish culture's eventual code is ku while on Ubuntu it's ckb-IQ. I am not sure how this is relevant to this though.

@neilboyd
Copy link
Contributor Author

neilboyd commented Feb 9, 2022

@clairernovotny I obviously wasn't suggesting to permanently skip the tests. That's why I put the PR back to draft until I could figure it out. I was just checking if this is the only reason the build is failing on windows-latest.

@mhmd-azeez Thanks for confirming the behavior I saw. I tried using UseCulture("ckb-IQ") and UseCulture("ku-Arab-IQ") but they also fail.

@neilboyd
Copy link
Contributor Author

neilboyd commented Feb 9, 2022

In Powershell, if I run Get-Culture -ListAvailable | ? DisplayName -match "Kurd", this is the output. There's no ku.

LCID             Name             DisplayName
----             ----             -----------
4096             ckb              Central Kurdish
4096             ckb-IQ           Central Kurdish (Iraq)
4096             ckb-IR           Central Kurdish (Iran)

@clairernovotny
Copy link
Member

If you run that in the windows-2019 agent, what does it return there? I'm trying to find a definitive source for what the locale code should be here and am struggling.

@neilboyd
Copy link
Contributor Author

neilboyd commented Feb 10, 2022

The tests succeed on the windows-2019 build agent. That was my first fix. If you want something quick to fix the build then that could be a temporary solution, but I don't think it's a good long term solution.

I'm trying to find the source of the issue. I don't think it's the build agent, or even dotnet cli, because they're just platforms for running other stuff.

What's also weird is that the failing tests depend on how it's built:

  • if I build and run with VS 2022 then all tests pass
  • if I build with VS 2022 and run tests with dotnet cli then all test pass
  • if I build with dotnet cli then DateHumanizeTests and TimeSpanHumanizeTests fail but NumberToWordsTests passes - that's the same whether I run the tests with dotnet cli or VS 2022

So that tells me that it's something to do with the dotnet cli build. Not the test runner, and not the installed locales.

@clairernovotny
Copy link
Member

Looks like we're hitting this issue: dotnet/msbuild#3897 that affects builds on .NET 6. Building in VS works because it's using .NET Framework.

@neilboyd neilboyd marked this pull request as ready for review February 11, 2022 06:04
@neilboyd
Copy link
Contributor Author

Looks like we're hitting this issue: dotnet/msbuild#3897 that affects builds on .NET 6. Building in VS works because it's using .NET Framework.

Yes, it looks like it. That issue is also referenced by dotnet/msbuild#7331

@neilboyd neilboyd changed the title Fix Kurdish locale in VS 2022 Use VSBuild instead of dotnet build Feb 11, 2022
@neilboyd
Copy link
Contributor Author

neilboyd commented Feb 15, 2022

@clairernovotny how do you want to proceed with this? Until something is done no-one can build any PRs.
Maybe the simplest fix is to keep using dotnet build, but tie the pipeline to windows-2019 until dotnet/msbuild#7331 is fixed.
That does mean that build.ps1 also fails, but I guess you can live with that.

I would suggest the following:

  1. change this PR back to use windows-2019 and dotnet build instead of VSBuild because that's the minimum change to keep things working as they were
  2. create a new issue here to change back to windows-latest once Satellite Assemblies not generated for some cultures (zh-CN) in .NET 6 dotnet/msbuild#7331 is fixed
  3. remove the line about running build.ps from guidelines and PR template since that's implicit anyway if the CI build is successful

@clairernovotny clairernovotny merged commit 17324e1 into Humanizr:main Feb 17, 2022
@clairernovotny
Copy link
Member

I'm still not sure if we need both ku and ckb but this will be good for now.

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.

3 participants