eunit: Pass node name to {node, ...} instantiator

[williamthome]

The migration from slave to peer in OTP-28 discarded the node name returned by peer:start_link/1, passing the Pid to the instantiator instead of the node name. This made {node, ...} unusable since the instantiator needs the node name to use with {spawn, Node, Tests} or erpc:call/4.

Capture the node name and wrap the instantiator so it receives the node name as an atom. Restore auto-start of net_kernel for non-distributed nodes so {node, ...} works out of the box.

https://erlangforums.com/t/eunit-node-instantiator-broken-in-otp-28-x/5465?u=williamthome

[github-actions]

CT Test Results

  2 files   12 suites   3m 18s ⏱️
110 tests 106 ✅ 4 💤 0 ❌
126 runs  122 ✅ 4 💤 0 ❌

Results for commit e02aa5c.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• No Windows Installer found

// Erlang/OTP Github Action Bot

[rlipscombe]

Looks good to me. I checked it with the following:

-module(node_test).
-include_lib("eunit/include/eunit.hrl").

node_spawn_test_() ->
    {node, 'foo', fun(Node) ->
        {spawn, Node, [
            {setup, fun remote_setup/0, [
                fun remote/0
            ]}
        ]}
    end}.

remote_setup() ->
    % uncomment to assert that the setup is running on the new node.
    % ?assertEqual(moo, node()),
    ok.

remote() ->
    ?assertEqual(moo, node()),
    ok.

[Mikaka27]

Is automatic enabling of net_kernel safe now? It was disabled on 27 as well.

[williamthome]

Is automatic enabling of net_kernel safe now? It was disabled on 27 as well.

Hmm I'm unsure. Where/when was it disabled in OTP 27? I've been trying to locate the specific change but haven't found it. Or do you mean the comments that already exist removed by this PR?

Note the auto-start is intentionally limited to EUnit's {node, ...} test context only and when node() is nonode@nohost, not a global re-enable. If there are specific safety concerns I should address, happy to add docs or a config flag.

Do you recommend removing the auto-start?

[rlipscombe]

Given that net_kernel:start/1 is deprecated -- per https://www.erlang.org/doc/apps/kernel/net_kernel.html#start/1 -- should this use net_kernel:start/2 instead?

[Mikaka27]

Yes, looks like you're right

[Mikaka27]

Is automatic enabling of net_kernel safe now? It was disabled on 27 as well.

Hmm I'm unsure. Where/when was it disabled in OTP 27? I've been trying to locate the specific change but haven't found it. Or do you mean the comments that already exist removed by this PR?

Note the auto-start is intentionally limited to EUnit's {node, ...} test context only and when node() is nonode@nohost, not a global re-enable. If there are specific safety concerns I should address, happy to add docs or a config flag.

Do you recommend removing the auto-start?

I meant the comment you removed, which is present on maint-27 branch as well. I don't know yet, will have to dig into the code and test if it's ok to do that.
Did you try running it with your change during system initialization? I think the comment is talking about various ways to execute code early as described here: https://www.erlang.org/doc/apps/erts/init.html

[williamthome]

Did you try running it with your change during system initialization? I think the comment is talking about various ways to execute code early as described here: https://www.erlang.org/doc/apps/erts/init.html

Yes, I tested it during system initialization using all init flags (-eval, -s, -run, -S) on a non-distributed node. All passed. I've added these as test cases in eunit_SUITE:node_test/1 to ensure this keeps working.

[Mikaka27]

Since this can be considered a bug, could you rebase your branch from patch-base-28 tag, so we can apply it to older branches? We will run this in our CI and see if there are any problems with this automatic start of net_kernel.

[williamthome]

could you rebase your branch from patch-base-28 tag

Yep, can do, but I didn't find this tag:

Or do you mean patch-base-27 tag or maint-28 branch?

[Mikaka27]

You're right, it doesn't exist, my bad. You can rebase from tag OTP 28.4

[williamthome]

You're right, it doesn't exist, my bad. You can rebase from tag OTP 28.4

Done.