Skip to content

Migrate Unit Tests to Xunit#130

Merged
TheCodeTraveler merged 11 commits intomainfrom
Fix-Unit-Tests
Oct 8, 2021
Merged

Migrate Unit Tests to Xunit#130
TheCodeTraveler merged 11 commits intomainfrom
Fix-Unit-Tests

Conversation

@TheCodeTraveler
Copy link
Copy Markdown
Collaborator

@TheCodeTraveler TheCodeTraveler commented Oct 7, 2021

Description

Migrating to Xunit is the first step towards adding Device Tests to the MAUI Community Toolkit.

To run Device Tests, XHarness is required, which uses Xunit. Writing both device and non-device unit tests with Xunit also allows us to re-use code between the testing projects.

Note: There is a known-issue with using dotnet test with a Unit Test that also includes `true in its csproj. The .NET MAUI team is aware of this and are targeting a fix for .NET MAUI Preview 10 in November.

Details

  • Remove NUnit NuGet Package
  • Add xunit NuGet Package
  • Convert NUnit attributes to xunit attributes, e.g. [Test] -> [Fact]
  • Remove Directory.Build.targets
    • This was causing an error because testhost.runtimeconfig.json was not included in the bin output
    • Directory.Build.targets was only necessary in earlier previews of .NET MAUI

@TheCodeTraveler TheCodeTraveler requested a review from pictos October 7, 2021 15:11
@TheCodeTraveler
Copy link
Copy Markdown
Collaborator Author

@jfversluis Could you take a look at why the PR Feed is failing?
image

Copy link
Copy Markdown
Member

@pictos pictos left a comment

Choose a reason for hiding this comment

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

I've just two comments here, but I think that doesn't block this PR


public void Dispose()
{
GC.SuppressFinalize(this);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is something that I'm not 100% sure about, should we call this method just if we implement the destructor (`~ BaseTest() {}`)?

Copy link
Copy Markdown
Collaborator Author

@TheCodeTraveler TheCodeTraveler Oct 8, 2021

Choose a reason for hiding this comment

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

I added IDisposible because it is recommended by Xunit as the replacement for [TearDown] in Nunit:
https://xunit.net/docs/comparisons

image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Other tests that also want to include [TearDown] code can override protected virtual void Dispose(bool isDisposing)

public class ExampleTest : BaseTest
{
    [Fact]
    public void ExampleTest()
    {
      // Test code
    }

    protected override void Dispose(bool isDisposing)
    {
        // Teardown test
        base.Dispose(isDisposing);
    }
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm sorry, I was talking about the GC.SuppressFinalize(this); It's fine to implement the Dispose method without that GC method, I guess. As I mentioned I need to review the pattern, but this shouldn't be a blocker anyway.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ah - gotchya.

Yea, GC.SupressFinalize was new to me too!

I found it recommended in the docs: https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose#the-dispose-method

@jfversluis
Copy link
Copy Markdown
Member

jfversluis commented Oct 8, 2021

@jfversluis Could you take a look at why the PR Feed is failing?

This one should be fixed from here on out!

Copy link
Copy Markdown
Member

@pictos pictos left a comment

Choose a reason for hiding this comment

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

LGTM!

@TheCodeTraveler TheCodeTraveler merged commit 7192d68 into main Oct 8, 2021
@delete-merged-branch delete-merged-branch bot deleted the Fix-Unit-Tests branch October 8, 2021 23:34
@github-actions github-actions bot locked and limited conversation to collaborators Nov 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants