[eunit] Slave module deprecation: Replace slave with peer in code and comments #10128

kvakvs · 2025-08-21T11:00:19Z

Eunit - Summary of changes

slave is removed from comments and private APIs as a unwelcome word.
New word is used for this, follower (or no word if removed)
To retain compatibility between old and new command line options formats (string versus list of separate strings) there is a command line parse function invoked on the old command line (old style tests assuming spawning using slave)

As slave module is also deprecated, usage of slave:start in the tests is updated to peer:start

github-actions · 2025-08-21T11:01:04Z

CT Test Results

2 files 12 suites 4m 7s ⏱️
109 tests 105 ✅ 4 💤 0 ❌
125 runs 121 ✅ 4 💤 0 ❌

Results for commit adc67fd.

♻️ 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

// Erlang/OTP Github Action Bot

Parse commandline function for eunit Test added and passing

CLAassistant · 2025-08-25T12:31:21Z

All committers have signed the CLA.

u3s · 2025-08-25T12:45:45Z

lib/eunit/doc/guides/chapter.md

@@ -1009,11 +1009,13 @@ The following representations specify fixture handling for test sets:

 - **`{node, Node::atom(), Tests | Instantiator}`**

- **`{node, Node::atom(), Args::string(), Tests | Instantiator}`** - `node` is
-  like `setup`, but with a built-in behaviour: it starts a slave node for the
+- **`{node, Node::atom(), Args::string()|[string()], Tests | Instantiator}`** - `node` is


follow convention and wrap string()|[string()] with space characters as on the right in same line, so it isstring() | [string]

u3s · 2025-08-25T13:03:58Z

lib/eunit/doc/guides/chapter.md

- **`{node, Node::atom(), Args::string(), Tests | Instantiator}`** - `node` is
-  like `setup`, but with a built-in behaviour: it starts a slave node for the
+- **`{node, Node::atom(), Args::string()|[string()], Tests | Instantiator}`** - `node` is
+  like `setup`, but with a built-in behaviour: it starts a follower node for the


naming is difficult :-)
I dislike follower term. arguments:

when used as "EUnit follower" providing additional context might be required for users to understand it

it suggest being passive and not doing job, one may think it is a monitor of some sort

is not aligned with common test

the less different terms to describe the same the better it is

the more self explanatory the term the better it is

I know common_test is not yet "slave free" but can we think about a term that would be shared between test frameworks?

I'm currently thinking about:

test node

peer test node

but maybe we can figure out something better.
@IngelaAndin can you have a look on this naming comment only? what is your opinion?

I agree that the use of follower is confusing term here. I think it feel like there is some new concept that is not being explained.

From doc.
A "fixture" is some state that is necessary for a particular set of tests to run. EUnit's support for fixtures makes it easy to set up such state locally for a test set, and automatically tear it down again when the test set is finished, regardless of the outcome (success, failures, timeouts, etc.).

And then
node is like setup, but with a built-in behaviour: it starts a slave node for the duration of the tests. The atom Node should have the format [email protected], and Args are the optional arguments to the new node; see [slave:start_link/3](https://www.erlang.org/docs/28/apps/stdlib/slave#start_link/3) for details.

In the context I think replacing slave with peer can suffice.

Fixture is commonly used as "small things set up (in the database or filesystem), necessary for running a test"
Calling a node 'fixture' would be confusing
Maybe use any word + node. Test node, peer node, follower node/leader node.

We discussed and this and we should go with peer (as that is what the replacement module is called, unless it is semantically incorrect).

u3s · 2025-08-25T13:18:06Z

lib/eunit/doc/guides/chapter.md

-  node; see `slave:start_link/3` for details.
+  node; see `peer:start_link/1` for details. For compatibility
+  reasons `Args` can be either a string or a list of strings, and if a single
+  string is passed, it will be parsed into a list with `argparse` module.


why mentioning argparse here at all?
is this needed?
where argparse is used in eunit code?
i can't find it, is it a left over and you went for implementing it yourself? why? am I missing something?

There isn't argparse, needs to be removed. I implemented the logic myself

slave starts node passing args as a string, peer starts node passing args as a list of strings. There needs to be support of string conversion to be passed as a list of args. Initially i wanted to use argparse and it was reflected in the docs, but now it is not used, so the mention was removed

yes, I understand that.
but why argparse is not enough in this case, I guess argparse might be capable of parsing the string but not necessarily producing list of strings as an output. but I just speculate here, wanted to learn the story :-)

u3s · 2025-08-25T13:21:54Z

lib/eunit/doc/overview.edoc

@@ -963,11 +985,13 @@ tests, with optional teardown afterwards. The arguments are described in
 detail below.
 </dd>
 <dt>`{node, Node::atom(), Tests | Instantiator}'</dt>
-<dt>`{node, Node::atom(), Args::string(), Tests | Instantiator}'</dt>
+<dt>`{node, Node::atom(), Args::string()|[string()], Tests | Instantiator}'</dt>


wrap | with spaces

u3s · 2025-08-25T13:36:32Z

lib/eunit/doc/guides/chapter.md

  duration of the tests. The atom `Node` should have the format
  `[email protected]`, and `Args` are the optional arguments to the new
-  node; see `slave:start_link/3` for details.
+  node; see `peer:start_link/1` for details. For compatibility
+  reasons `Args` can be either a string or a list of strings, and if a single


I understand we have:

new type expected by peer module and recommended by us (make it 1st on lists?), as less processing is performed when used

a legacy type kept for compatibility reasons but always converted to format preferred by peer

maybe above is the information we would like to share with users. so that those who read docs, understand better what they should use to get best results.

From the "processing cost" perspective it is negligible. Breaking a list into words is cheap. I can emphasize this dual type of node arguments, that new type is list of strings, and string is just supported to not break old tests (from the experience some tests will still break but hopefully users will find the explanation in this doc paragraph we are editing now).

yes. but even if negligible, users might understand why 2 possibilities exist and which is preferred these days.

I changed the wording in markdown docs and edoc yesterday, please have a look

u3s · 2025-08-25T13:51:41Z

lib/eunit/src/eunit_data.erl

+-spec parse_peer_args(string() | [string()]) -> [string()].
+parse_peer_args([]) -> [];
+parse_peer_args(Args) when is_list(Args) -> % can be string or list of strings
+    IsCodepoint = fun(CP) -> is_integer(CP) andalso 0 =< CP andalso CP < 16#110000 end,


why not rely on one of io_lib:printable* functions? maybe we could then avoid this code?

u3s · 2025-08-25T14:44:22Z

pls. squash commits before merging.

kvakvs requested a review from u3s August 21, 2025 11:00

kvakvs added team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI labels Aug 21, 2025

kikofernandez removed the testing currently being tested, tag is used by OTP internal CI label Aug 21, 2025

kvakvs added the testing currently being tested, tag is used by OTP internal CI label Aug 21, 2025

kikofernandez removed the testing currently being tested, tag is used by OTP internal CI label Aug 21, 2025

kvakvs force-pushed the eimytdl/slave-to-peer-eunit/OTP-19738 branch from 3055ad2 to 14eaba3 Compare August 25, 2025 01:09

IngelaAndin assigned kvakvs Aug 25, 2025

kvakvs added 4 commits August 25, 2025 14:30

Replace slave:start and 'slave' in eunit

99da6f0

Parse commandline function for eunit Test added and passing

Dialyzer fix

5ee251e

Satisfy dialyzer

26b890d

Edoc license header

f3e2aba

kvakvs force-pushed the eimytdl/slave-to-peer-eunit/OTP-19738 branch from c7f029d to f3e2aba Compare August 25, 2025 12:31

u3s requested changes Aug 25, 2025

View reviewed changes

IngelaAndin mentioned this pull request Aug 26, 2025

[mnesia] Slave module deprecation: Replace slave to peer in code and comments #10070

Merged

Documentation adjustments; Use io_lib instead of unicode lib

adc67fd

IngelaAndin requested review from IngelaAndin and u3s September 2, 2025 12:12

IngelaAndin changed the title ~~[eunit] Slave module deprecation: Replace slave to follower in code and comments~~ [eunit] Slave module deprecation: Replace slave with peer in code and comments Sep 8, 2025

[eunit] Slave module deprecation: Replace slave with peer in code and comments #10128

Are you sure you want to change the base?

[eunit] Slave module deprecation: Replace slave with peer in code and comments #10128

Uh oh!

Conversation

kvakvs commented Aug 21, 2025

Eunit - Summary of changes

Uh oh!

github-actions bot commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CT Test Results

Artifacts

Uh oh!

CLAassistant commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

u3s commented Aug 25, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 21, 2025 •

edited

Loading

CLAassistant commented Aug 25, 2025 •

edited

Loading