Skip to content

Modify MNTR to work with the new MNTR package #5288

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 28, 2021
Merged

Modify MNTR to work with the new MNTR package #5288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89aba34
Remove fact command line argument dependency
Arkatufus Sep 27, 2021
f13af0f
Fix CommandLine, should not depend on Environment.GetCommandLineArgs
Arkatufus Sep 27, 2021
89730cf
Fix old MNTR to run with the new fact attribute and CommandLine
Arkatufus Sep 28, 2021
6ac62bc
Merge branch 'dev' into Multinode_migration
Arkatufus Sep 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/core/Akka.MultiNodeTestRunner.Shared.Tests/MultiNodeTestRunnerDiscovery/DiscoverySpec.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Akka.Remote.TestKit;
using Akka.TestKit;
using Xunit;
using FluentAssertions;
Expand Down Expand Up @@ -62,6 +63,7 @@ public void Only_MultiNodeConfig_role_count_used()

private static Dictionary<string, List<NodeTest>> DiscoverSpecs()
{
Environment.SetEnvironmentVariable(MultiNodeFactAttribute.MultiNodeTestEnvironmentName, "1");
Copy link
Member

Choose a reason for hiding this comment

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

What's this for?

Copy link
Member

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

using (var controller = new XunitFrontController(AppDomainSupport.IfAvailable, new System.Uri(typeof(DiscoveryCases).GetTypeInfo().Assembly.CodeBase).LocalPath))
{
using (var discovery = new Discovery())
Expand Down
5 changes: 3 additions & 2 deletions src/core/Akka.MultiNodeTestRunner/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ class Program
/// </summary>
static void Main(string[] args)
{
// Force load the args
CommandLine.GetPropertyOrDefault("force load", null);
CommandLine.Initialize(args);

if (CommandLine.ShowHelp)
{
PrintHelp();
Expand Down Expand Up @@ -242,6 +242,7 @@ static void Main(string[] args)
}
#endif

Environment.SetEnvironmentVariable(MultiNodeFactAttribute.MultiNodeTestEnvironmentName, "1");
using (var controller = new XunitFrontController(AppDomainSupport.IfAvailable, assemblyPath))
{
using (var discovery = new Discovery())
Expand Down
3 changes: 3 additions & 0 deletions src/core/Akka.NodeTestRunner/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class Program

static int Main(string[] args)
{
CommandLine.Initialize(args);

var nodeIndex = CommandLine.GetInt32("multinode.index");
var nodeRole = CommandLine.GetProperty("multinode.role");
var assemblyFileName = CommandLine.GetProperty("multinode.test-assembly");
Expand Down Expand Up @@ -64,6 +66,7 @@ static int Main(string[] args)
#endif

Thread.Sleep(TimeSpan.FromSeconds(10));
Environment.SetEnvironmentVariable(MultiNodeFactAttribute.MultiNodeTestEnvironmentName, "1");
using (var controller = new XunitFrontController(AppDomainSupport.IfAvailable, assemblyFileName))
{
/* need to pass in just the assembly name to Discovery, not the full path
Expand Down
12 changes: 7 additions & 5 deletions src/core/Akka.Remote.TestKit/CommandLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,19 @@ namespace Akka.Remote.TestKit
/// var testName = CommandLine.GetProperty("multinode.test-method");
/// </code>
/// </summary>
public class CommandLine
public static class CommandLine
{
private static readonly StringDictionary Values;
private static readonly StringDictionary Values = new StringDictionary();

static CommandLine()
{
Values = new StringDictionary();
Initialize(Environment.GetCommandLineArgs());
Copy link
Contributor Author

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.

Copy link
Member

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...

}

public static void Initialize(string[] args)
Copy link
Contributor Author

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.

{
// Detect and fix PowerShell command line input.
// PowerShell splits command line arguments on '.'
var args = Environment.GetCommandLineArgs();
var fixedArgs = new List<string>();
for (var i = 1; i < args.Length - 1; ++i)
{
Expand Down Expand Up @@ -71,7 +73,7 @@ static CommandLine()

if (tokens.Length == 2)
{
Values.Add(tokens[0], tokens[1]);
Values[tokens[0]] = tokens[1].Trim().Trim('"');
}
else
{
Expand Down
18 changes: 6 additions & 12 deletions src/core/Akka.Remote.TestKit/MultiNodeFact.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,17 @@ public class MultiNodeFactAttribute : FactAttribute
/// Set by MultiNodeTestRunner when running multi-node tests
/// </summary>
public const string MultiNodeTestEnvironmentName = "__AKKA_MULTI_NODE_ENVIRONMENT";

public static Lazy<bool> ExecutedByMultiNodeRunner =
new Lazy<bool>(() =>
{
var args = Environment.GetCommandLineArgs();
if (args.Length == 0) return false;
var firstArg = args[0];
return firstArg.Contains("Akka.MultiNodeTestRunner")
Copy link
Contributor Author

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.

|| firstArg.Contains("Akka.NodeTestRunner")
|| Environment.GetEnvironmentVariable(MultiNodeTestEnvironmentName) != null;
});

private bool? _executedByMultiNodeRunner;

public override string Skip
{
get
{
return ExecutedByMultiNodeRunner.Value
if (_executedByMultiNodeRunner == null)
_executedByMultiNodeRunner =
Environment.GetEnvironmentVariable(MultiNodeTestEnvironmentName) != null;
return _executedByMultiNodeRunner != null && _executedByMultiNodeRunner.Value
? base.Skip
: "Must be executed by multi-node test runner";
}
Expand Down