test: twister: -s path requires forward slashes on Windows, didn't before.

[glenn-andrews]

Using Windows, I had a test that I was running of the following form:
west twister --device-testing --device-serial COM3 --device-serial-baud 115200 -p disco_l475_iot1 -T C:\Users\glenn\zephyrproject\my_app\ -s tests\app\sample.testing.my_app_tests

After #64272 the direction of the path separators in the -s option changed without warning:

west twister --device-testing --device-serial COM3 --device-serial-baud 115200 -p disco_l475_iot1 -T C:\Users\glenn\zephyrproject\my_app\ -s tests\app\sample.testing.my_app_tests
Renaming output directory to C:\Users\glenn\zephyrproject\my_app\twister-out.18
INFO    - Using Ninja..
INFO    - Zephyr version: zephyr-v3.5.0-1349-gdbed25124906
INFO    - Using 'zephyr' toolchain.
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\glenn\zephyrproject\.venv\Scripts\west.exe\__main__.py", line 7, in <module>
  File "C:\Users\glenn\zephyrproject\.venv\Lib\site-packages\west\app\main.py", line 1085, in main
    app.run(argv or sys.argv[1:])
  File "C:\Users\glenn\zephyrproject\.venv\Lib\site-packages\west\app\main.py", line 244, in run
    self.run_command(argv, early_args)
  File "C:\Users\glenn\zephyrproject\.venv\Lib\site-packages\west\app\main.py", line 505, in run_command
    self.run_extension(args.command, argv)
  File "C:\Users\glenn\zephyrproject\.venv\Lib\site-packages\west\app\main.py", line 654, in run_extension
    self.cmd.run(args, unknown, self.topdir, manifest=self.manifest,
  File "C:\Users\glenn\zephyrproject\.venv\Lib\site-packages\west\commands.py", line 194, in run
    self.do_run(args, unknown)
  File "C:\Users\glenn\zephyrproject\zephyr\scripts\west_commands\twister_cmd.py", line 60, in do_run
    ret = main(options)
          ^^^^^^^^^^^^^
  File "C:\Users\glenn\zephyrproject\zephyr\scripts\pylib\twister\twisterlib\twister_main.py", line 120, in main
    tplan.discover()
  File "C:\Users\glenn\zephyrproject\zephyr\scripts\pylib\twister\twisterlib\testplan.py", line 180, in discover
    raise TwisterRuntimeError("No test cases found at the specified location...")
twisterlib.error.TwisterRuntimeError: No test cases found at the specified location...

The test did work when the path separators are replaced with / e.g.:

west twister --device-testing --device-serial COM3 --device-serial-baud 115200 -p disco_l475_iot1 -T C:\Users\glenn\zephyrproject\my_app\ -s tests/app/sample.testing.my_app_tests

It would be nice if it was fixed to work with slashes in either direction.

Impact:
Annoyance. I was stuck for several hours until I bisected the repo and found the diff that changed the separator direction. There's no indication of what caused the problem, and older tests will now fail on Windows when they worked before.

Environment:
Windows 11.
zephyr 0.16.5
Commit: any after #64272

[hakehuang]

@glenn-andrews as the \ is used so can you try with
west twister --device-testing --device-serial COM3 --device-serial-baud 115200 -p disco_l475_iot1 -T C:\Users\glenn\zephyrproject\my_app\ -s tests/app/sample.testing.my_app_tests

[glenn-andrews]

@hakehuang yes, it works. I know it works. I said above that it works.

The entire point of this issue is that in making it work, the PR broke the previous method of specifying the argument to -s on Windows, which was to use \ to match the path specifier passed to -T

[hakehuang]

Windows, which was to use \ to match the path specifier passed to -T

because, we meet a problem when build from linux like system, but run on windows system. so I propse this add as '/' is accepted by both Windows and Linux, so we assume use will use it this way. @nashif, do you think we can unified the -s content to '/'?

[glenn-andrews]

Windows, which was to use \ to match the path specifier passed to -T

because, we meet a problem when build from linux like system, but run on windows system. so I propse this add as '/' is accepted by both Windows and Linux, so we assume use will use it this way. @nashif, do you think we can unified the -s content to '/'?

So your proposal is to break every test that uses \ without any explanation to the user why their tests suddenly broke? Especially since -T allows paths to use both separators?

twisterlib.error.TwisterRuntimeError: No test cases found at the specified location... does not explain the problem is Twister suddenly changed the path separator. Why not either:

1. Sanitize the input by searching for \ and converting to / internally?
2. Check for the presence of \ and tell the user it's an invalid path separator rather than print a meaningless error?
3. Add a note to the documentation that all path separators in Twister must use Linux path separators on Windows?

[hakehuang]

So your proposal is to break every test that uses \ without any explanation to the user why their tests suddenly broke? Especially since -T allows paths to use both separators?

sorry, what I mean is exact solution 1 you propsed. forgive me for ambious description, so the code change is simple as below.

diff --git a/scripts/pylib/twister/twisterlib/environment.py b/scripts/pylib/twister/twisterlib/environment.py
index 0d14345317a..6f17d151eb7 100644
--- a/scripts/pylib/twister/twisterlib/environment.py
+++ b/scripts/pylib/twister/twisterlib/environment.py
@@ -752,7 +752,8 @@ def parse_arguments(parser, args, options = None):
         # directly.
         if options.test:
             for scenario in options.test:
-                if dirname := os.path.dirname(scenario):
+                normalized_scenario = scenario.replace(os.sep, '/')
+                if dirname := os.path.dirname(normalized_scenario):
                     options.testsuite_root.append(dirname)

and for

Add a note to the documentation that all path separators in Twister must use Linux path separators on Windows?

'/' is a path separator accepted by both OS, that;s why we select this as default one, but you are right, we need document this.

[nashif]

@hakehuang assigning this to you, given you were involved in introduing this issue :-)

[glenn-andrews]

Thank you. Sorry if I was harsh. I had two Twister bugs simultaneously occur, preventing any forward motion until I bisected from 3.5.0 to the present to find both of them.

I appreciate your working to fix this.

[PerMac]

@nashif @hakehuang @glenn-andrews
TL;DR: I don't like option 1. I believe options 2 and 3 is the way to go.
IMO it was an issue before which was fixed by Hake with normalizing the paths. It is twister feature that test identifiers contain paths by default. The paths have different separators on widows and posix. But the test identifier shouldn't change depending on the OS IMO. This is the same test, regardless of the os, and should have the same identifier. Converting \ to / in the backend might be more convenient to users in short term, but can cause further confusion, e.g. when dealing with test management tools, comparing results, etc. @glenn-andrews have you tried the option I recommended --no-detailed-test-id? With it paths are not included in the identifiers and you should be able to completely drop them from -s args. Explaining the above in the dosc can be valuable. Detecting usage of "" in -s and printing a warning can also help

[glenn-andrews]

@nashif @hakehuang @glenn-andrews TL;DR: I don't like option 1. I believe options 2 and 3 is the way to go. IMO it was an issue before which was fixed by Hake with normalizing the paths. It is twister feature that test identifiers contain paths by default. The paths have different separators on widows and posix. But the test identifier shouldn't change depending on the OS IMO. This is the same test, regardless of the os, and should have the same identifier. Converting \ to / in the backend might be more convenient to users in short term, but can cause further confusion, e.g. when dealing with test management tools, comparing results, etc. @glenn-andrews have you tried the option I recommended --no-detailed-test-id? With it paths are not included in the identifiers and you should be able to completely drop them from -s args. Explaining the above in the dosc can be valuable. Detecting usage of "" in -s and printing a warning can also help

--no-detailed-test-id works and completely misses the point.

I lost 2 days of work because I copied tests that worked the previous month to test in a new project, and when they failed with no intelligent error message I assumed it was an error on my part, not that someone had submitted a PR that swapped the direction of the path separator.

I personally don't care if it's backslashes, forward slashes or both. I do care I lost two days of productivity to an undocumented breaking change, and I doubt I'm the only developer out there who used the Windows path separators on Windows and found it blew up in their face next time they pulled the latest version.

Given so many other things in Zephyr don't care if you use Windows or Linux path separators, why are we hung up on making sure only Linux separators work? Why not swap all \ to / for all OS and then if you want to use Linux separators in windows, or windows separators in MacOS, it Just Works.

[hakehuang]

Given so many other things in Zephyr don't care if you use Windows or Linux path separators, why are we hung up on making sure only Linux separators work? Why not swap all \ to / for all OS and then if you want to use Linux separators in windows, or windows separators in MacOS, it Just Works.

In my fix PR, all path has been pre normized with '/', please check. Thanks a lot.

[glenn-andrews]

In my fix PR, all path has been pre normized with '/', please check. Thanks a lot.

It works for me on Windows.

[glenn-andrews]

Fixed with #70468