Taking a step back, what are these tests checking for? Maybe they should be updated to instead check for the full name? They may have only been checking for just the base name of the node because that's all the old behavior gave.

I think that checking for the full name is probably the best idea. But that will require me to make quite a few more changes. I'll get started.

Ok, sorry, wasn't trying to invent work for you, but I think it would be better to refresh these particular tests. Thanks.

No, no! It's A-OK! Though, I'm having a bit of difficulty updating the tests - it seems some of the test files are actually generated, and I'm not positive how to set up the generated node names. I'll push my latest changes in a sec.

EDIT Apologies for the word salad, I was in a bit of a rush. It seems that the tests are generated from the py.in files. I'm just trying to find where these files get their executable_args. Looking at the output of these tests, it seems that the original author is using the same string to both generate a node name and look for it. Which is fine normally, but the name qualification process tacks a "/" on the front of the name.

I'd like for the arguments to be correct from the get-go, rather than have the test manually add a "/" on the front of a passed in name.

I just bit the bullet and implemented the string modification in the test. We needed to cast argv[1] to a std::string anyways.

Is there a preferred way to do this? Can I get a CI on this?

@wjwwood Do the changes look acceptable?

CI (just test_rclcpp on windows since there are comments to address before full CI)

Okay, made the changes you asked for. However, it looks like the testing API has gotten changed? Do I need to make any changes to account for this, or should it just work?

Because it seems like it's done something funky to my fastrps setup... The tests have suddenly stopped working reliably.

Edit Nevermind, they seem to have sorted themselves out?

CI looks good ros2/rclcpp#698 (comment)