Skip to content

Commit

Permalink
Merge pull request #421: Clean up functional test MacTODO categories …
Browse files Browse the repository at this point in the history
…and Windows-only option

Prior to starting work to support runs of the functional test suite on Linux, we can simplify some of the tests marked `MacTODO` as no longer needing that designation and also simplify some of the Windows-related configuration and options.

On the Windows side of thing, we can remove the second invocation of `excludeCategories.Add(Categories.MacOnly)` because it is always performed on Windows in the `if/else` block just above it.

We also only parse the `--windows-only` functional test command-line option when running on Windows, as it has no effect on macOS since the `WindowsOnly` tests are always excluded there.

On the macOS side of things, there is one test still marked `MacTODO.NeedsServiceVerb` which appears to have been missed in PR #216, where that category was removed from many other tests.  This `SecondCloneSucceedsWithMissingTrees` test succeeds on macOS as written, so it seems OK to remove the `MacTODO` flag on it.

The other macOS change we make is to remove the `MacTODO.NeedsScalarConfig` category, which currently applies to all the tests in `MultiEnlistmentTests/ConfigVerbTests.cs`.  These tests are also marked with `NeedsUpdatesForNonVirtualizedMode` and therefore will continue to _not_ run even after this change.  (However, limited testing by disabling the `config` verb's "elevated privileges" [checks](https://github.com/microsoft/scalar/blob/ef5c1bc753d0e129a2ca27c0e4daee78a27d1d30/Scalar/CommandLine/ConfigVerb.cs#L95) and removing `NeedsUpdatesForNonVirtualizedMode` from the `ConfigVerbTests` indicates that, in fact, these tests would all succeed on macOS/Linux with some further adjustments.  But for now we just remove the "extra" `MacTODO` exclusion on these tests.)

These two changes allow us to remove two of the three `MacTODO` categories entirely.

We also do minor housekeeping by fixing one typo in the Watchman `shutdown-server` command.
  • Loading branch information
derrickstolee authored Aug 17, 2020
2 parents 2af74c4 + d68c5af commit 245d7ee
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 25 deletions.
6 changes: 0 additions & 6 deletions Scalar.FunctionalTests/Categories.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ public static class Categories

public static class MacTODO
{
// Tests that require Config to be built
public const string NeedsScalarConfig = "NeedsConfig";

// Tests that require Scalar Service
public const string NeedsServiceVerb = "NeedsServiceVerb";

// Tests requires code updates so that we lock the file instead of looking for a .lock file
public const string TestNeedsToLockFile = "TestNeedsToLockFile";
}
Expand Down
27 changes: 10 additions & 17 deletions Scalar.FunctionalTests/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,36 +76,29 @@ public static void Main(string[] args)
ScalarTestConfig.FileSystemRunners = FileSystemRunners.FileSystemRunner.DefaultRunners;
}

if (runner.HasCustomArg("--windows-only"))
{
includeCategories.Add(Categories.WindowsOnly);

// RunTests unions all includeCategories. Remove ExtraCoverage to
// ensure that we only run tests flagged as WindowsOnly
includeCategories.Remove(Categories.ExtraCoverage);
}

if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
excludeCategories.Add(Categories.MacTODO.NeedsScalarConfig);
excludeCategories.Add(Categories.MacTODO.NeedsServiceVerb);
excludeCategories.Add(Categories.MacTODO.TestNeedsToLockFile);
excludeCategories.Add(Categories.WindowsOnly);
}
else
{
if (runner.HasCustomArg("--windows-only"))
{
includeCategories.Add(Categories.WindowsOnly);

// RunTests unions all includeCategories. Remove ExtraCoverage to
// ensure that we only run tests flagged as WindowsOnly
includeCategories.Remove(Categories.ExtraCoverage);
}

excludeCategories.Add(Categories.MacOnly);
}

// For now, run all of the tests not flagged as needing to be updated to work
// with the non-virtualized solution
excludeCategories.Add(Categories.NeedsUpdatesForNonVirtualizedMode);

if (!RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
excludeCategories.Add(Categories.MacOnly);
}

ScalarTestConfig.RepoToClone =
runner.GetCustomArgWithParam("--repo-to-clone")
?? Properties.Settings.Default.RepoToClone;
Expand All @@ -117,7 +110,7 @@ public static void Main(string[] args)
{
// Shutdown the watchman server now that the tests are complete.
// Allows deleting the unwatched directories.
ProcessHelper.Run("watchman", "shudown-server");
ProcessHelper.Run("watchman", "shutdown-server");
}
catch (Exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ namespace Scalar.FunctionalTests.Tests.MultiEnlistmentTests
{
[TestFixture]
[Category(Categories.ExtraCoverage)]
[Category(Categories.MacTODO.NeedsScalarConfig)]
[Category(Categories.NeedsUpdatesForNonVirtualizedMode)]
public class ConfigVerbTests : TestsWithMultiEnlistment
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public void GitObjectsRecreatedWhenDownloadingObjects()
}

[TestCase]
[Category(Categories.MacTODO.NeedsServiceVerb)]
public void SecondCloneSucceedsWithMissingTrees()
{
string newCachePath = Path.Combine(this.localCacheParentPath, ".customScalarCache2");
Expand Down

0 comments on commit 245d7ee

Please sign in to comment.