-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Modify MNTR to work with the new MNTR package #5288
Conversation
|
||
static CommandLine() | ||
{ | ||
Values = new StringDictionary(); | ||
Initialize(Environment.GetCommandLineArgs()); |
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.
Have to do this, else net472 tests would fail. It seems like static scope in net472 is different than in netcore, static somehow didn't get propagated from the node runner to the xunit test runner.
var args = Environment.GetCommandLineArgs(); | ||
if (args.Length == 0) return false; | ||
var firstArg = args[0]; | ||
return firstArg.Contains("Akka.MultiNodeTestRunner") |
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.
Dropped because we're not using separate executable to run the nodes anymore, and we also have a different namespace/package name that conflicts these. We'll rely on the environment variable to detect whether a test is being run inside MNTR or not.
|
||
public static void Initialize(string[] args) |
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.
Have to be able to inject arguments from other sources than Environment.GetCommandLineArgs()
since we're not relying on CLI args anymore.
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.
LGTM
@@ -62,6 +63,7 @@ public void Only_MultiNodeConfig_role_count_used() | |||
|
|||
private static Dictionary<string, List<NodeTest>> DiscoverSpecs() | |||
{ | |||
Environment.SetEnvironmentVariable(MultiNodeFactAttribute.MultiNodeTestEnvironmentName, "1"); |
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.
What's this for?
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.
"Flag to mark that the current test is being run inside the MNTR" according to @Arkatufus in chat - got it
|
||
static CommandLine() | ||
{ | ||
Values = new StringDictionary(); | ||
Initialize(Environment.GetCommandLineArgs()); |
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.
Ahhh got it - need to defer the Environment.GetCommandLineArgs
call until we're ready...
This completed a full run of the MNTR and everything ran successfully by the looks of it. LGTM |
No description provided.