Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Skipping Admin-only tests instead of failing them #766

Merged
merged 7 commits into from
Jun 15, 2019
Merged

Skipping Admin-only tests instead of failing them #766

merged 7 commits into from
Jun 15, 2019

Conversation

agr
Copy link
Contributor

@agr agr commented Jun 13, 2019

Got fed up with all the redness in test explorer. This introduces two additional test attributes that will automagically skip if tests are executed not as admin. Since there were two test assemblies with admin-only tests, had to introduce shared test library with common code.

Tested in VS: when run as regular user, skips all tests that failed before, when run as admin runs them fine. Tested that CI still runs those tests after the change.

To enable test skip one needs to add the ENABLE_NONADMIN_TEST_SKIP environment variable. This was done to reduce the chance of accidentally skipping those tests in CI (in initial implementation they would be skipped if CI started to run tests as non-admin).

Copy link
Contributor

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

Awesome idea 👍

{
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
[XunitTestCaseDiscoverer("Xunit.Sdk.FactDiscoverer", "xunit.execution.{Platform}")]
public class FactIfAdminAttribute : FactAttribute
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any name suggestions? AdminOnlyFact maybe?

Copy link
Member

Choose a reason for hiding this comment

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

I like AdminOnly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have theories, too, also AdminOnly does not emphasize that it's a test.

…MIN_TEST_SKIP` environment variable so that there would be no way to skip those in CI by accident.
@agr agr merged commit a48031f into dev Jun 15, 2019
@agr agr deleted the agr-admin-tests branch June 15, 2019 00:08
joelverhagen pushed a commit that referenced this pull request Jul 31, 2020
The `auxiliary2azuresearch` job needs to know which popularity transfers have changed to properly update the search index.

Previous change: NuGet/NuGet.Services.Metadata#765
Part of NuGet/NuGetGallery#7898
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
* Skipping admin-only tests when run as non-admin gracefully instead of failing them.
* Made non-admin test skip conditional on the presence of `ENABLE_NONADMIN_TEST_SKIP` environment variable so that there would be no way to skip those in CI by accident.
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
The `auxiliary2azuresearch` job needs to know which popularity transfers have changed to properly update the search index.

Previous change: NuGet/NuGet.Services.Metadata#765
Part of NuGet/NuGetGallery#7898
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