Sitelet https://github.com/DiscUtils/DiscUtils/pull/37
Skip to content

enable unittests for net45 - #37

Merged
qmfrederik merged 6 commits into
DiscUtils:masterfrom
ahdde:unittest
Dec 22, 2017
Merged

qmfrederik merged 6 commits into
DiscUtils:masterfrom
ahdde:unittest

Conversation

@zivillian

Copy link
Copy Markdown
Contributor

This PR enables unit test support for the net452 framework (the lowest supported by xunit 2.2.0). Since the xunit runner is signed, I had to disable PublisSign (at least for the windows platform) - otherwise the clr would refuse to load the assemblies.

As an optimization I've also disable the creation of nuget packages for debug builds (to speed up build and test time).

The is still an issue with the sample data tests for xfs and lvm2 failing with an OutOfMemoryException on 32Bit under net452. As a workaround the runner can be started as 64bit using the following commandline:

dotnet test Tests\LibraryTests\LibraryTests.csproj -c Debug --logger:trx -f net452 -- RunConfiguration.TargetPlatform="x64"

There is also an option for VS.

To prevent a possibly broken CI I've modified the appveyor.yml to run only the netcoreapp1.1 tests.

There are still many known problems with xunit and net452 like having to delete temporary folders and restarting VS to be able to run those tests from within VS, but those are out of scope for this project.

@LordMike

Copy link
Copy Markdown
Member
  • Wouldn't it refuse to load the assemblies because the test project isn't signed? I don't think we can disable signing entirely, as the nuget packages should be signed at least. I could go with not signing in Debug if it's a problem.
    • In fact.. How come the runner is signed - i thought strong name signed assemblies could only load other SN signed assemblies ... An unsigned assembly should never have a problem loading signed assemblies though.
  • Great optimization reg. nuget on debug
  • If we support net4x tests, they should be run in CI. I assume AV's image (VS2017 I think) should support both core and net4x in the same go. So it shouldn't be too hard - I might look at that one.

@zivillian

Copy link
Copy Markdown
Contributor Author

To be clear: I only disabled PublicSign - SignAssembly is still set to true. So the assemblies are still signed.
I don't have enough knowledge about signing, to tell the exact difference between SignAssembly and PublicSign and which problems may arise from each option.

The condition was actually stolen from an asp.net core example (or the MS docs), but I have no idea, what the reason behind this condition is since I was not able to find any useful documentation.

@zivillian zivillian mentioned this pull request Aug 1, 2017
@zivillian

Copy link
Copy Markdown
Contributor Author

I've rebased onto #60 and also enabled the unit tests for net45 in appveyor.

There is still an issue with parallel test execution on net45 (thus the failing build) - I will look into this.

@zivillian

Copy link
Copy Markdown
Contributor Author

The last commit disables parallel test execution, which fixes the build. Since the whole discutils project is not thread safe, I guess there is no benefit in trying to make the collections of registered modules thread safe.

@qmfrederik
qmfrederik merged commit 130e456 into DiscUtils:master Dec 22, 2017
@qmfrederik

Copy link
Copy Markdown
Contributor

Thanks!

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