-
Notifications
You must be signed in to change notification settings - Fork 254
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
Removed Discover Warnings in Test Output | Issue #437 #480
Removed Discover Warnings in Test Output | Issue #437 #480
Conversation
@parrainc : Thank you for your contribution but unfortunately we want to have test classes to work only with public access modifier and not with private/internal for performance reasons. |
Hi @jayaranigarg thanks for your comment. But, notice that the behaviour will be the same for discovering tests (just public test classes), the only thing I changed is that internal/private test classes won't report in the output, as stated the expected behaviour in the issue. In any case, if you have suggestions that I can follow, so I can update the PR, please let me know. |
@jayaranigarg just following this one up... wanted to know if there's suggestions on what needs to be modified in this PR. |
@parrainc : We indeed want internal/private classes to be reported in the output, otherwise a user will not be able to figure out the reason why his/her tests are not getting discovered. We somehow just want to suppress these warnings for our unit-test classes, where we don't want testmethods to run as such as they are dummy classes which are merely used as assets in writing unit tests. |
Gotcha! I misunderstood the issue a little bit. @jayaranigarg |
@parrainc : That's great! Let us know once you push in the new changes. |
This reverts commit bc4ccd7.
…e437-discoverwarnings
@jayaranigarg new changes are ready for review (with some delay). Please take a look when available and let me know your thoughts :) |
@@ -412,6 +412,10 @@ public void TestMethod() | |||
} | |||
} | |||
|
|||
private class DummyTestClassAttribute : UTF.TestClassAttribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add [DummyTestClass] to class DummyTestClassWithCleanupMethods(Line 388) as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did it, but when added the [DummyTestClass]
attribute to DummyTestClassWithCleanupMethods
class, one of the unit test failed RunCleanupShouldReturnCleanupResultsForAssemblyAndCla ssCleanupMethods
because of the AssemblyCleanup method is not being called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed it is. I did even try debugging the unit test that was using the DummyTestClassWithCleanupMethods
with the [DummyTestClass]
and [UTF.TestClass]
but could not observe nothing. and got same behavior except for the part of the assemblyCleanup thing. Notice that when invoking the ClassCleanup it works properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried debugging this. Basically, when we get DeclaredMethods for DummyTestClassWithCleanupMethods
, it is not returning AssemblyCleanup() method in the list of methods for that type and hence AssemblyCleanup() is not getting called. Now, the question is why AssemblyCleanup() is not present in the list when we mention [DummyTestClass]
and is present in case of [UTF.TestClass]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the same question I asked to myself...
I created a mstest project, added a class CustomTestClass
that inherits from TestClass
, and set up the assembly/class initalize and cleanup methods. It looks like there could be an issue when a test class is inheriting from a custom attribute that inherits from TestClassAttribute.
Have you seem any issues regarding to that?
Here's the debug output with CustomTestClass
attribute:
expectedValue
is a private static int field declared in the test class
ClassInitialize: before -> 0
ClassInitialize: after -> 321
TestMethod: expectedValue -> 321Assert.AreEqual failed. Expected:<123>. Actual:<321>.
ClassCleanup: expectedValue -> 321
notice that with the custom attribute, neither the assembly initialize nor the cleanup were executed
Here's the debug output with TestClass
attribute:
AssemblyInitialize: before -> 0
AssemblyInitialize: after -> 123
ClassInitialize: before -> 123
ClassInitialize: after -> 123
TestMethod: expectedValue -> 123
ClassCleanup: expectedValue -> 123
AssemblyCleanup: expectedValue -> 123
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jayaranigarg any updates?
I'm thinking of opening an issue with my inputs in above comments? Would that be alright?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parrainc : That should be fine. Please raise a separate issue for this and let's get this PR in.
@parrainc : LGTM 👍 Just a minor comment. Please address that and it should be good to push in. |
This PR fixes the issue #437. After building the solution and the test discover starts, non-public tests won't report in the Test output.
[Update]
Created a "dummy test class attribute" that inherit from TestClassAttribute to be used in the dummy test classes that were generating warnings in the Output.
This is how is looks after these changes: