Net6launch #297

josephmyers · 2022-09-16T05:48:25Z

This change is

This reverts commit 14bf423.

This reverts commit 57ab2ec.

Seeing what happens when the hgrc always points to ChorusMerge.exe over the script. Based on the comment, it would appear chorusmerge was only necessary for Mono. It would be really nice to be able to eliminate that extra piece from the equation, but it's near impossible to test without pushing packages to Nuget. Also not clear is why the hgrc is even using the chorusmerge script, when that set lies in IsMono code. Is Platform.IsMono true when on .NET6?

currently run conditions are still master or manual run

This is an effort to reproduce the chorusmerge script failure on .NET6. The script may be being called by LfMerge, but something about the ChorusMerge executable has changed between Framework and .NET6. It could be a pathing issue. I noticed that when I mangled the executable name in the hgrc, the "hg merge" command failed silently, as it does in production. Now, I seem to remember verifying chorusmerge wasn't even being launched in the lfmerge container. However, with the changes in ExecutionEnvironment, we don't use chorusmerge in the first place, which would be a great improvement if it's possible. Sending this to the other computer, since this one's WSL is hosed.

This is required on .NET6 to prevent an exception.

However, instead of triggering off of Platform.IsMono, we trigger off of Platform.IsLinux. It's not Mono that makes the script required, at least apparently. hg isn't able to launch the .exe directly on Linux. Someone with more Linux experience may be able to configure it to do so, but for the current effort I'm re-integrating the chorusmerge script.

is this the easiest way to test linux? ugh

github-actions · 2022-09-16T05:53:15Z

Test Results

    10 files +      2   443 suites +204 1h 26m 28s ⏱️ + 40m 49s
  973 tests +      1   917 ✔️ ±      0   56 💤 +    1 0 ❌ ±0
4 190 runs +1 992 3 966 ✔️ +1 887 224 💤 +105 0 ❌ ±0

Results for commit 37e6d0b. ± Comparison against base commit 9549415.

This pull request skips 1 test.

ExternalFileModification_NotifiesIndices_ButSaveDoesNot

♻️ This comment has been updated with latest results.

Got the test failure I was expecting. The test chorusmerge is being called. This is different from how I remember production behaving. I wasn't able to verify chorusmerge is being called in the production environment. But there could've been something else going on.

Plus better print statements for the test chorusmerge

With .IsMono, the test, according to the output statements, is failing like the production environment. So that's a good step

.github/workflows/dotnet.yml

src/LibChorus/Utilities/ExecutionEnvironment.cs

dotnet run is not working, which is what's in the production code

trying to get dotnet pack to increment. when it's the same name/version, something in the process causes it to be ignored, even though the output dll's are different

One note: System.Diagnostics.Debug no longer exists in .NET Core. System.Diagnostics.Trace is more or less equivalent for the purposes of the test using it. We'll see how the build server responds.

Also only running the corresponding tests on Framework. Technically, this does leave a coverage gap for .NET/Windows.

…eption on Linux/.NET6

It's certainly possible this actually specific to Mono, but I think it in fact applies to Linux as a whole, and the previous was a slight oversight. We'll see what the build server thinks.

MergeConflictEmbeddedMessageContentHandler is dependent on WinForms, and as such we shouldn't expect it to be present on .NET6 builds. The Platform("Net-4.0") indicates a run on Windows/net461 and Linux/mono/net461.

This test is supposed to be skipped on the build server. Clearly it is not. Ugh.

it's hanging. maybe due to net6?

super confused. latest changes from develop are breaking on this branch

josephmyers · 2022-09-20T04:23:53Z

I'm not sure what's going with the latest changes to dotnet.yml, but they're causing the windows tests (net461) to hang. Before, the command was run: dotnet test -- NUnit.TestOutputXml=TestResults, but now that it's run: dotnet test --no-build -c Release --filter TestCategory!=SkipOnBuildServer -- NUnit.TestOutputXml=TestResults, it hangs during a test.

Here's a snippet from a test run without --no-build:

  Skipped LaunchDialog_SimulatedUsb_USBHasInvalidRepo
  Skipped ShowIt
  Skipped LaunchDialog_CustomAddress
  Skipped LaunchDialog_FullAddress
Warning: Killing Hg Process...

    Syncing...
    Started at 2022-09-20 03:55:53Z
    Local User: bob
    LanguageForge User:

But with it, it hangs after Skipped LaunchDialog_FullAddress.

Unfortunately, removing --no-build avoids the hang, but now ChorusMerge.Tests fail and LibChorus.Tests are skipped. Ideas, anyone?

This reverts commit d2732d8.

This reduces clarity of what would be happening in this case, but I want to see if it's causing the unit test hang on build server.

This reverts commit fd3a105.

This reverts commit 6ae4705.

How could this be related? I'm out of ideas

ermshiperete · 2022-09-21T08:26:11Z

Do you know which tests hang? Did you try to ignore (for the purpose of tracking down the problem) the new HgMergeTests to see if that makes a difference?

josephmyers · 2022-09-21T08:30:36Z

Even with a diagnostic test run, the current test being run isn't printed (lol), but I figured it out and have the fix pushed. It was related to some Encoding call required for .NET6 but breaking for Framework.

The tests take longer to finish, now that we're running LibChorus on .NET6 as well. I think it's worth it, but I'm open to whatever.

src/ChorusMerge/ChorusMerge.csproj

src/ChorusMerge/Program.cs

src/LibChorusTests/MiscTests.cs

src/LibChorusTests/VcsDrivers/Mercurial/HgMergeTests.cs

ermshiperete · 2022-09-21T10:54:30Z

src/LibChorus/Model/ServerSettingsModel.cs

-			var encryptedData = ProtectedData.Protect(Encoding.Unicode.GetBytes(encryptMe), Encoding.Unicode.GetBytes(EntropyValue), DataProtectionScope.CurrentUser);
+
+			if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) &&
+				!RuntimeInformation.FrameworkDescription.Contains("Framework"))


This doesn't work when testing on my local machine. When running with Mono RuntimeInformation.FrameworkDescription returns Mono 6.12.0.182 (tarball Tue Jun 14 22:35:00 UTC 2022).

When running the net6.0 tests in Rider things don't work either. I created a libpalaso PR (sillsdev/libpalaso#1227) that makes things behave correctly.

Ok, I'm unable to see this on my Linux box. Should I replace Framework with something? Maybe Mono? Do I need another condition? Or I could try deleting it?

What command are you using to run with Mono? It's quite possible I'm not using the right thing

I reverted this code. It's unrelated to the core bug, which was just Platform.IsMono -> .IsLinux, and created a separate issue for it.

I was stepping through the net461 tests in the debugger in Rider on Ubuntu 22.04.

Ok. Yeah I have less of a pure Linux environment, being on WSL, so I trust your PC more than mine

…t work

I noticed they weren't running on the build server, and we have precedent for the Mono keyword on other Platform attributes.

josephmyers and others added 12 commits September 9, 2022 11:04

tailoring workflow for testing

57ab2ec

oh and only run on linux

14bf423

Revert "oh and only run on linux"

5a5821a

This reverts commit 14bf423.

Revert "tailoring workflow for testing"

49b953a

This reverts commit 57ab2ec.

publish to nuget on all check-in types

b2f71a7

currently run conditions are still master or manual run

Registering 1252 encoding.

4589516

This is required on .NET6 to prevent an exception.

Merge remote-tracking branch 'origin/net6launch' into net6launch

c893895

just run one important test

59492a0

is this the easiest way to test linux? ugh

Merge branch 'master' into net6launch

0fb4ba7

josephmyers added 5 commits September 16, 2022 13:01

Merge remote-tracking branch 'origin/net6launch' into net6launch

7cbd005

Have to use dotnet run for Framework .exe

0bc2d92

Reverting temporarily back to Platform.IsMono to verify the test breaks.

9cd8b22

Plus better print statements for the test chorusmerge

Going back to Platform.IsLinux

4a7d95f

With .IsMono, the test, according to the output statements, is failing like the production environment. So that's a good step

ermshiperete requested changes Sep 16, 2022

View reviewed changes

.github/workflows/dotnet.yml Outdated Show resolved Hide resolved

src/LibChorus/Utilities/ExecutionEnvironment.cs Outdated Show resolved Hide resolved

josephmyers and others added 11 commits September 16, 2022 13:29

Trying to get ChorusMerge.exe to exec

5a0eead

dotnet run is not working, which is what's in the production code

Print statements in LibChorus

262fe19

trying to get dotnet pack to increment. when it's the same name/version, something in the process causes it to be ignored, even though the output dll's are different

Cleaning up.

a69b62e

One note: System.Diagnostics.Debug no longer exists in .NET Core. System.Diagnostics.Trace is more or less equivalent for the purposes of the test using it. We'll see how the build server responds.

Missed some things in dotnet.yml

e8fc714

Fixed the mono execution for the chorusmerge test script

07817dd

Throw exception for .NET6/Linux for password encryption/decryption.

2f32cc6

Also only running the corresponding tests on Framework. Technically, this does leave a coverage gap for .NET/Windows.

Allowing null/empty passwords for encrypt/decrypt before throwing exc…

0479781

…eption on Linux/.NET6

Fixing a test based on the recent Platform.IsMono -> .IsLinux change

4a2dbcf

It's certainly possible this actually specific to Mono, but I think it in fact applies to Linux as a whole, and the previous was a slight oversight. We'll see what the build server thinks.

Excluding WinForms-based test on .NET6

bf7ec13

MergeConflictEmbeddedMessageContentHandler is dependent on WinForms, and as such we shouldn't expect it to be present on .NET6 builds. The Platform("Net-4.0") indicates a run on Windows/net461 and Linux/mono/net461.

Excluding another stupid test for .NET6

0f09ff8

This test is supposed to be skipped on the build server. Clearly it is not. Ugh.

trying to fix test run on windows

2902344

it's hanging. maybe due to net6?

adding linux back

13ba463

super confused. latest changes from develop are breaking on this branch

josephmyers marked this pull request as ready for review September 20, 2022 04:14

josephmyers requested a review from ermshiperete September 20, 2022 04:14

josephmyers and others added 11 commits September 20, 2022 13:34

Testing if removing net6 helps the net461 hang...

d2732d8

Merge remote-tracking branch 'origin/net6launch' into net6launch

4927e4d

let's run windows first

0ef9f7f

Revert "Testing if removing net6 helps the net461 hang..."

3cef566

This reverts commit d2732d8.

Merge remote-tracking branch 'origin/net6launch' into net6launch

bf3627a

Reverting unsupported exception throw in ServerSettingsModel.

fd3a105

This reduces clarity of what would be happening in this case, but I want to see if it's causing the unit test hang on build server.

Revert "Reverting unsupported exception throw in ServerSettingsModel."

fc18f09

This reverts commit fd3a105.

diag tests

6ae4705

Revert "diag tests"

7e12442

This reverts commit 6ae4705.

Reverting something else

269fe3b

How could this be related? I'm out of ideas

Adding back the Encoding register, but only for .NET6+Windows

37e6d0b

josephmyers requested review from papeh and jasonleenaylor September 21, 2022 08:51

ermshiperete reviewed Sep 21, 2022

View reviewed changes

Merging a some test lines for PR comment

98dd9e3

josephmyers requested a review from ermshiperete September 22, 2022 02:24

josephmyers and others added 3 commits September 22, 2022 10:45

Replacing the exception throw with a comment, since the former doesn'…

1b7233b

…t work

Adding Mono to tests recently excluded for .NET6.

5c4fbb3

I noticed they weren't running on the build server, and we have precedent for the Mono keyword on other Platform attributes.

reverting GHA test change

c7dafd8

ermshiperete approved these changes Sep 22, 2022

View reviewed changes

ermshiperete mentioned this pull request Sep 22, 2022

Chorus fails to resolve merge conflict when syncing in Language Forge #294

Closed

josephmyers linked an issue Sep 22, 2022 that may be closed by this pull request

Chorus fails to resolve merge conflict when syncing in Language Forge #294

Closed

josephmyers merged commit 7d1b3e3 into sillsdev:master Sep 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Net6launch #297

Net6launch #297

josephmyers commented Sep 16, 2022 •

edited by jasonleenaylor

Loading

github-actions bot commented Sep 16, 2022 •

edited

Loading

josephmyers commented Sep 20, 2022

ermshiperete commented Sep 21, 2022

josephmyers commented Sep 21, 2022

ermshiperete Sep 21, 2022 •

edited

Loading

josephmyers Sep 22, 2022

josephmyers Sep 22, 2022

josephmyers Sep 22, 2022

ermshiperete Sep 22, 2022

josephmyers Sep 22, 2022

Net6launch #297

Net6launch #297

Conversation

josephmyers commented Sep 16, 2022 • edited by jasonleenaylor Loading

github-actions bot commented Sep 16, 2022 • edited Loading

Test Results

josephmyers commented Sep 20, 2022

ermshiperete commented Sep 21, 2022

josephmyers commented Sep 21, 2022

ermshiperete Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

josephmyers Sep 22, 2022

Choose a reason for hiding this comment

josephmyers Sep 22, 2022

Choose a reason for hiding this comment

josephmyers Sep 22, 2022

Choose a reason for hiding this comment

ermshiperete Sep 22, 2022

Choose a reason for hiding this comment

josephmyers Sep 22, 2022

Choose a reason for hiding this comment

josephmyers commented Sep 16, 2022 •

edited by jasonleenaylor

Loading

github-actions bot commented Sep 16, 2022 •

edited

Loading

ermshiperete Sep 21, 2022 •

edited

Loading