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

Consider switching to a different test framework #420

Closed
gvheertum opened this issue Feb 20, 2019 · 10 comments
Closed

Consider switching to a different test framework #420

gvheertum opened this issue Feb 20, 2019 · 10 comments
Labels

Comments

@gvheertum
Copy link
Member

Currently the MSTest framework is used in the unit tests. However, this might be limiting for more complex/extensive tests.

When writing my testcases for the lexer I missed attributes like [Theory] (XUnit) or [TestCase] (NUnit), that would have allowed us to have testcases instead of asserting multiple smaller elements in 1 test.

This code

        [TestMethod]
        public void Lexer_ImportantAfterHex_Success()
        {
            //Important should be valid on 3 digit hex
            GenerateLexerTestFile("border-top: 1px solid #333 !important;");
            //Important should be valid on 6 digit hex
            GenerateLexerTestFile("border-top: 1px solid #009c46 !important;");
            //Whitespace should not break the parser
            GenerateLexerTestFile("border-bottom: 1px solid #009c46  ;");
        }

would become

        [TestCase("border-top: 1px solid #333 !important;")] //Important should be valid on 3 digit hex
        [TestCase("border-top: 1px solid #009c46 !important;")]  //Important should be valid on 6 digit hex
        [TestCase("border-bottom: 1px solid #009c46  ;")]   //Whitespace should not break the parser
        public void Lexer_ImportantAfterHex_Success(string contentToTest)
        {
            GenerateLexerTestFile(contentToTest);
        }

If one of the cases fails, the rest of the cases will still be tested, in the old scenario the first failing Assert/call will stop execution of the other cases.

@mrbean-bremen
Copy link
Member

Agreed. I'm also not very happy with the testing framework, having used nunit before.
As always - any help is welcome!

@gvheertum
Copy link
Member Author

If you are okay with NUnit, I can try setting up a migration to NUnit. xUnit is a bit quircky if you are not used to it.

@mrbean-bremen
Copy link
Member

That would be nice! I'm not very proficient with NUnit, even if have used it - currently I'm mostly working with Pytest and Google test...

@mrbean-bremen
Copy link
Member

And of course we should check with @tebjan if this is OK, as this would be an infrastructure change that may have some impact.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Jun 17, 2019

@gvheertum - are you still planning to work on this? I'm really not very happy with MSTest...
@tebjan - would that be ok (e.g. switching to NUnit)? If yes, and @gvheertum doesn't have the time, I may have a go at this myself.

@gvheertum
Copy link
Member Author

@mrbean-bremen If we have a formal go (or if @tebjan doesn't have a problem), I can give it a try. The conversion from MSTest to NUnit is relatively easy (getting the most out of the framework is a different story, but getting it replaced is easy). I'll try to create a PR this week.

@tebjan
Copy link
Contributor

tebjan commented Jun 18, 2019

@gvheertum sounds great, i fully support this idea!

@gvheertum
Copy link
Member Author

@tebjan Thanks, I'm on it. Locally I already have the test adapter changed to NUnit without too much problems. Only issue is with the image test which uses the CSV, that needs some additional work, but I hope to be able to put in a PR tomorrow.

@gvheertum
Copy link
Member Author

PR #493 has a change proposal for the migration to NUnit, including some cleanups on tests since they could be refactored due to the new framework.

@gvheertum
Copy link
Member Author

Code was merged, so this issue is resolved :)

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

No branches or pull requests

3 participants