Skip to content

Conversation

@NickCraver
Copy link
Collaborator

Before converting most of the project to C# 7, netstandard2.0, etc. I want to get unit tests in shape. There's a fair bit of complication to doing so in that we need to test against:

  • Different versions
  • Clusters
  • Master/Slave setups
  • Multiple-test unfriendly tests (like SHUTDOWN commands)
  • Simulate remote (read: latent) connections

This work does a few things:

  • Moves all unit tests and assertions to xUnit
  • Enables parallelization for tests
  • Utilizes the xUnit output semantics, which means things like connection logs are on the test in VS, console output, build servers, etc. (it was a giant pool of Console.WriteLine mixed before)
  • Cleans up the \packages folder completely (it wasn't used for anything but the redis-*.exe binaries
  • Cleans up all test code formatting in general
  • Re-enables net462 in testing (was only netcoreapp1.0 before)

Still TODO

  • Better handling of Skips (e.g. if you aren't running clusters or 4.0 locally)
    • [Theory] skips
    • Feature-based skips
    • SSL-based skips
  • Analyze all current failures and fix up if possible

Long-term TODOs

  • Look at Redis 4.0 on Windows via WSL
  • Configure endpoints in a .json config override that's in .gitignore
  • Find a way to handle master/slave breaks on underpowered machines better

Posting this for review as I go, hopefully if people have time to eyeball they can spot existing errors with unit tests as well. Certainly far more cleanup can be done, and comments are appreciated.

No need for all these when NETStandard.Library is in play.
For now, also uses local host as the remote, we need a more multi-user friendly structure for this (and to move Booksleeve tests under the main project).
Not perfect, more tweaks to ordering and parallelization to make. When running on an underpowered maching (e.g. this laptop), Redis servers lose connection.

Enhancements:
- dotnet xunit works
- Tests are run in parallel
- Current CLI Counts: Total: 730, Errors: 0, Failed: 105
  - VS includes and passes more, see: parallization conflicts and CPU starvation above
- Output is now properly in the test cases (rather than a ton of Console.WriteLine) for much easier debugging, e.g. every test has a nicely formatted and output connection log, and only when the test fails (unless diagnostic logging is enabled via argument or runner config)
- Lots of formatting fixes
- Initial Skippable tests implemented

TODOs:
- Need skips for Cluster, SSL, and latest features
- Need skips for Theory (assuming you can do this)
- Move redis binaries to get rid of \packages to avoid confusion
Also removes /packages to eliminate any confusion.
Restores redis-cli.exe as well, not sure what happened there.
Let's get some initial testing back online, I hope.
This will let us skip based on feature sets the same way everywhere, e.g. when testing against an older server.

This also makes the base [Fact] and [Theory] skippable, meaning no custom attributes everywhere.
Breaking Master/Slave setup obviusly screws a ton of other tests, so we need to isolate these at the end of the run.
This appears to be broken for the Sync and Async type, but getting more data logged for fixing it.
NickCraver and others added 26 commits August 27, 2017 20:22
Previously, tests would fail if the computer didn't have a D:\ drive at all (e.g. this laptop...)

Now, tests have a default config in TestConfig. To override anything, use a _TestConfig.json file (interntionally excluded from git) to specify things like RedisLabs and SSL servers. Or generally any non-localhost style configuration. This should be the 1 place to add endpoints, for example if we add some IPv6 tests. It also makes it easier to swap tests to be against another endpoint like Azure, off-site test clusters, etc.
Some test don't actually need 1 master to check anything valid, so make them cope with the tests that split better.
Moving to an embedded resource approach given how xUnit command line runs.
These tests take several minutes each. To re-enable, use _TestConfig.json like this:
{
  "RunLongRunning": true
}
Sentinel moved to skip (unless configured) since we don't have a local setup/run script yet. Will get to that at the end.
Aggregate nonsense otherwise when we're looking at specific throw types.
Some of the global exceptions can be tracked per-instance (those created on the multiplexer), so do that. This reduces cross-erroring of tests for ambient exceptions known from the multiplexer.
These needed updating for multiple servers in the default test
multiplexer
@NickCraver
Copy link
Collaborator Author

Merging into master to continue work on other PRs and getting decent CI validation up. Remaining tests are: "I don't think the test is wrong" failures I need to get with Marc on. The landscape has changed a bit.

@NickCraver NickCraver merged commit 48a0a99 into master Aug 30, 2017
@NickCraver NickCraver deleted the craver/xunit2 branch May 27, 2018 13:32
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.

2 participants