vunit icon indicating copy to clipboard operation
vunit copied to clipboard

library.get_test_benches() behaves differently on Windows and Linux

Open endrebjorsvik opened this issue 4 years ago • 2 comments

The pattern matching in ui.library.get_test_benches() behaves slightly different on Window and Linux. When I give it the exact name of a test bench without leading wildcards, it does not find any test benches on Linux. On Windows, it finds the test bench. The following Python snippet summarize my findings:

from vunit import VUnitCLI, VUnit

cli = VUnitCLI()
args = cli.parse_args()
vu = VUnit.from_args(args=args)

mylib = vu.add_library("mylib")
mylib.add_source_files("*/*.vhd")

for tb in mylib.get_test_benches("tb_module_a*"):
    print("Found testbench:", tb.name)

vu.main()
Linux:
$ python run.py -l
Traceback (most recent call last):
  File "run.py", line 14, in <module>
    for tb in mylib.get_test_benches("tb_module_a*"):
  File "/home/endre/src/venv/dig_asic/lib/python3.8/site-packages/vunit/ui/library.py", line 338, in get_test_benches
    return check_not_empty(
  File "/home/endre/src/venv/dig_asic/lib/python3.8/site-packages/vunit/ui/common.py", line 52, in check_not_empty
    raise ValueError(error_msg + ". Use allow_empty=True to avoid exception.")
ValueError: No test benches found within library mylib. Use allow_empty=True to avoid exception.

Windows:
> python run.py -l
Found testbench: tb_module_a
mylib.tb_module_a.mytest
Listed 1 tests

The directory is setup as follows:

| - run.py
\ - module_a
  | - module_a.vhd
  \ - tb_module_a.vhd

I have narrowed it down to the following check in library: https://github.com/VUnit/vunit/blob/8c670cf156478d718325b07628fda00c3b941c7d/vunit/ui/library.py#L333 pathlib.Path("foo").resolve() behaves slightly different on Windows and Linux. On Windows, the path remains unresolved if it does not exist. On Linux, the path is resolved to a full path regardless:

Linux:
>>> pathlib.Path("foo").resolve()
PosixPath('/full/path/to/foo')

Windows:
>>> pathlib.Path("foo").resolve()
WindowsPath("foo")

This difference makes the exact pattern fail on Linux, while is succeeds on Windows.

I would love to contribute with some sort of fix. However, it is not clear to me what is the intended behaviour of this pattern matching. Is it required to have a leading wildcard in the pattern, or is it also supposed to work without one? I see that the former implementation used os.path.abspath(), which resolves to a full path on both Windows and Linux. What is the purpose of resolving the test bench name to an absolute path in any of the cases?

This was tested on Python 3.8 on Windows 10 and CentOS 8, using VUnit v4.4.0.

endrebjorsvik avatar Nov 23 '20 19:11 endrebjorsvik

@endrebjorsvik thanks for that nice report!

The purpose when changing from os.path to pathlib.Path was not to change the behaviour. Therefore, I'd say this is an undesired backwards breaking change. If we required the wildcard before, we should fix it so that it's required now. Then, we can discuss about the need for resolving the absolute path or not; which might need a deeper understanding of the effects. Do you want to submit a PR that checks Path(test_bench.name).exists() or Path(test_bench.name).resolve().exists()?

eine avatar Nov 24 '20 04:11 eine

@eine Thanks for the clarification! I will try to submit a PR. I just need some time to get up to speed with the project test environment first.

endrebjorsvik avatar Dec 06 '20 22:12 endrebjorsvik