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

Ms test adapter #21

Merged
merged 8 commits into from
Jul 26, 2021
Merged

Ms test adapter #21

merged 8 commits into from
Jul 26, 2021

Conversation

Siphonophora
Copy link
Contributor

@Siphonophora Siphonophora commented May 25, 2021

Fixes #20

@codito This seems to fix the issue.

  • I have a decent bit of trouble seeing what going on inside the loggers when we get issues like this about data being formatted incorrectly. And it seems even more cumbersome now that we have this project. So I added the beginning of a debug mode to help with that. Do you think it makes sense to have the DebugLogger flag setup as public static? I know you use ConsoleOutput in some testing, but i'm not sure if we need this here. I can flesh out some other places that might be good to have logging. I would probably add some logging to the junit logger as well.
  • I'm interested in what you think of the changes to TestResultInfo. Is there a reason you left TestResult as private on this class? If it was public, the MSTestAdapter could do the buisness logic, instead of having the internal method I added. Seems like it would be cleaner to make TestResult public.

This is what I used as a source of paramaterized tests. Grabbed some of it from https://www.automatetheplanet.com/mstest-cheat-sheet/ so its hopefully covering all the cases.

    [TestClass]
    public class UnitTest1
    {
        public static IEnumerable<object[]> GetData()
        {
            yield return new object[] { 1, 1, 2 };
            yield return new object[] { 12, 30, 42 };
            yield return new object[] { 14, 1, 15 };
        }

        [TestMethod]
        public void TestMethod1()
        {
        }

        [DataTestMethod]
        [DataRow(true)]
        [DataRow(false)]
        public void Some_Test_Method(bool result)
        {
        }

        [DataTestMethod]
        [DataRow(true)]
        [DataRow(false)]
        public async Task Some_Async_Test_Method(bool result)
        {
        }

        [DataTestMethod]
        [DataRow(-1, "a")]
        [DataRow(0, "a")]
        [DataRow(1, "acsdf")]
        public void IsPrime_ValuesLessThan2_ReturnFalse(int value, string a)
        {
        }

        [DataTestMethod]
        [DynamicData(nameof(GetData), DynamicDataSourceType.Method)]
        public void TestAddDynamicDataMethod(int a, int b, int expected)
        {
        }

        [DataSource("Microsoft.VisualStudio.TestTools.DataSource.CSV", "TestsData.csv", "TestsData#csv", DataAccessMethod.Sequential)]
        [TestMethod]
        public void DataDrivenTest()
        {
            // NOTE this isn't parameterized, but its also getting skipped.
        }

@codito
Copy link
Contributor

codito commented Jun 1, 2021

@Siphonophora thank you for adding the debug logger. I think its a great idea. Let me think through the design and get back on this.

@Siphonophora
Copy link
Contributor Author

@codito Sounds good. I don't mind if we want to disentangle the debug from the mstest issue. That is a bug across all the loggers, so maybe a little more urgent.

@Siphonophora
Copy link
Contributor Author

@codito Have you thought at all about the logger? I'm starting to look into an older issue on junit and it seems like logging will help a lot there.

In any case, it seems like we should probably pull the logger from this and get the mstest fix released?

@codito
Copy link
Contributor

codito commented Jul 4, 2021

Sorry for the delay.

In any case, it seems like we should probably pull the logger from this and get the mstest fix released?

+1 to separating the logger. I think the logger may be a higher priority need since it is impacting your productivity (let me know if I got the priority order incorrect). Let's get that released right away. Two thoughts on the logger:

  1. Could we expose logger as part of ITestRun e.g. ITestRun.IDebugLogger? It will be available to most of the workflows, and if needed we can pass the reference to data structures like TestResultInfo. This will help inject a testable/noop logger for unit tests.
  2. Instead of exposing a key specific to our TestLoggers, can we depend on the test platform trace infra? See https://github.com/microsoft/vstest-docs/blob/main/docs/diagnose.md#collect-traces-using-command-line and the way to use it will be to use EqtTrace.* methods in our IDebugLogger implementation. See https://github.com/microsoft/vstest/blob/main/src/Microsoft.TestPlatform.CoreUtilities/Tracing/EqtTrace.cs#L24. All logs will get written into the log.txt we provide while invoking dotnet test.

I'm curious if the debugging TestLogger will be solved if we include the symbols together with nuget package (portable symbols or otherwise), that will enable stepping into code just like we could do earlier with JunitTestLogger itself. What do you think?

@Siphonophora Siphonophora mentioned this pull request Jul 25, 2021
@codecov
Copy link

codecov bot commented Jul 25, 2021

Codecov Report

Merging #21 (6c721fb) into master (365fc23) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #21   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           19        20    +1     
  Lines          503       522   +19     
=========================================
+ Hits           503       522   +19     
Impacted Files Coverage Δ
src/TestLogger/Core/TestCaseNameParser.cs 100.00% <ø> (ø)
src/TestLogger/TestLogger.cs 100.00% <ø> (ø)
src/TestLogger/Core/TestResultInfo.cs 100.00% <100.00%> (ø)
src/TestLogger/Extensions/MSTestAdapter.cs 100.00% <100.00%> (ø)
src/TestLogger/Extensions/TestAdapterFactory.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 365fc23...6c721fb. Read the comment docs.

@Siphonophora Siphonophora marked this pull request as ready for review July 25, 2021 17:58
@codito codito merged commit 86d5ab9 into spekt:master Jul 26, 2021
@Siphonophora Siphonophora deleted the MSTest-Adapter branch July 26, 2021 16:12
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.

MSTest parameterized test data
2 participants