-
Notifications
You must be signed in to change notification settings - Fork 509
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
Switch to Xunit #517
Switch to Xunit #517
Conversation
using StyleCop.Analyzers.DocumentationRules; | ||
using TestHelper; | ||
|
||
/// <summary> | ||
/// This class contains unit tests for <see cref="SA1600ElementsMustBeDocumented"/>- | ||
/// </summary> | ||
[TestClass] | ||
[Collection("FooCollection")] |
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.
❗ Let's remove the [Collection]
attribute for now.
In addition to the items mentioned in the diff:
|
@pdelvo There is no need to update appveyor.yml. This pull request is almost ready to merge if you just fix the few items marked with ❗ and nothing else. 👍 (I want to keep the changes simple since the diff is already big and checking it again is more work than average.) |
I implemented you the feedback you provided but there are still tests that dont run on my machine when I use the test explorer. I dont think I missed a TestMethod attribute. I think the build failed because it was doing a merged build which then includes some more TestMethods. Should I rebase the changes on master and fix these or are you fixing them when you merge? |
Found the bug in xunit (see the link). Should we do something about it? |
I fixed the remaining issue by renaming some conflicting methods. |
Do not merge.
I have some issues with the built in test explorer in VS. It does not run all the tests. 72 of them just stay as "Not Run Tests". The console runner however works fine. Maybe this is an issue with my machine.
I have not updated the appveyor file too.