VHDL / Compliance-Tests

Tests to evaluate the support of VHDL 2008 and VHDL 2019 features
https://vhdl.github.io/Compliance-Tests/
Apache License 2.0
25 stars 7 forks source link

Use absolute path to test006a.txt #40

Closed nickg closed 1 year ago

nickg commented 1 year ago

When run through VUnit the working directory is not vhdl_2019.

nickg commented 1 year ago

@bpadalino

umarcor commented 1 year ago

For context, see http://vunit.github.io/run/user_guide.html#special-paths and http://vunit.github.io/user_guide.html#special-generics-parameters.

In this case, I believe it makes sense to use the VHDL-2019 feature. Yet, those VUnit features might be useful for similar cases in VHDL-2008 tests.

nickg commented 1 year ago

One other small issue with this test is that it writes to the input file test006a.txt so the second time you run it it fails. Perhaps the test should regenerate that file each time, but that can be fixed separately.

LarsAsplund commented 1 year ago

The structural change looks good to me. However, it fails on file size. Probably it has to do with how newlines are dealt with. Maybe easier to just have a single line and avoid such problems? When I change that it passes in Riviera-PRO

nickg commented 1 year ago

However, it fails on file size. Probably it has to do with how newlines are dealt with. Maybe easier to just have a single line and avoid such problems? When I change that it passes in Riviera-PRO

Is that because you've run it once and the input file has been modified? It works for me with NVC as long as I do git checkout vhdl_2019/test006a.txt before running, although I haven't tested on Windows.

LarsAsplund commented 1 year ago

No, it doesn't get to the point where it starts modifying the file. This is a problem of its own though. We don't want a test that fails the second time.

I'm on Windows and

image

With three lines there would be three extra bytes on Windows,

nickg commented 1 year ago

I changed the test to regenerate the file each time. I'm curious if that works with Riviera Pro on Windows - NVC always opens files in binary mode so it wouldn't translate the newline to a CRLF pair but I can't remember what the LRM says here.

LarsAsplund commented 1 year ago

I'm not sure about the LRM either but in case of Riviera-PRO it saves the file with Windows line endings and file size is 25 bytes. Maybe you can have a reference file for testing file_size and then create a file dynamically for the other tests. To prevent git from messing with the reference file have a look at: https://stackoverflow.com/questions/10357436/how-to-make-git-not-change-line-endings-for-one-particular-file

JimLewis commented 1 year ago

I would expect that files have an appropriate newline for the OS on which they are running. So I would expect a tool running on Linux to produce linux newline and the same tool running on windows to produce a windows newline. So it will be different for windows and linux. Further, if one uses textio, then the read should remove the newline from the line - and I think in this case, it should remove either a windows or a linux newline - what ever happens to be in the file.

LarsAsplund commented 1 year ago

@JimLewis That would be the reasonable approach and I can't recall that I've seen any other behavior. The problem we face here is more of a git issue.

I looked up what the LRM says:

The language does not define the representation of the end of a line. An implementation shall allow all possible values of types CHARACTER and STRING to be written to a file. However, as an implementation is permitted to use certain values of types CHARACTER and STRING as line delimiters, it might not be possible to read these values from a TEXT file.

As a consequence, file_size is implementation dependent and we can't hard-code any expectations into our tests,

nickg commented 1 year ago

I changed it to write a literal string using plain file write instead of writeline. @LarsAsplund can you try that on Windows?

LarsAsplund commented 1 year ago

Interesting. The result is a text file with proper line endings but Riviera-PRO adds a magic 4-byte word to the beginning so the file size is 26. The LRM allows for special characters as line delimiters and I guess the beginning of a line may have those as much as the end of a line. Here is a HEX view of the file

image

0x16 is the synchronous idle character. Not sure if that has a history of being used in this way or if this is Riviera-PRO proprietary

LarsAsplund commented 1 year ago

If we have no way of knowing in advance the size of a file given what the LRM allows an implementation to do, we could check-in a text file in the repo like we did originally, and then let the run script determine the file size and pass that as a golden reference to the testbench via a generic. Even better, we can create the file in Python. No need to have it in the repo.

JimLewis commented 1 year ago

Is there anything that requires that  VHDLs implicitly defined write to file using file type TEXT to be ASCII.   I thought ASCII was guaranteed only when Textio write was used.Sent from my T-Mobile 4G LTE Device -------- Original message --------From: Lars Asplund @.> Date: 3/13/23 11:04 PM (GMT+01:00) To: VHDL/Compliance-Tests @.> Cc: Jim Lewis @.>, Mention @.> Subject: Re: [VHDL/Compliance-Tests] Use absolute path to test006a.txt (PR #40) Interesting. The result is a text file with proper line endings but Riviera-PRO adds a magic 4-byte word to the beginning so the file size is 26. The LRM allows for special characters as line delimiters and I guess the beginning of a line may have those as much as the end of a line. Here is a HEX view of the file

0x16 is the synchronous idle character. Not sure if that has a history of being used in this way or if this is Riviera-PRO proprietary

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

LarsAsplund commented 1 year ago

You're right @JimLewis , I can't find anything in the LRM specifying the format for such a file.

Just realized that 0x16 = 22 is the length of the string in the file.

If I create my own write function based on characters, I get the expected output:

    type text_file_t is file of character;

    procedure write(file_name, content : string) is
      file f : text_file_t;
    begin
      file_open(f, file_name, WRITE_MODE);
      write(f, content(content'left));
      file_close(f);

      file_open(f, file_name, APPEND_MODE);
      for idx in content'left + 1 to content'right loop
        write(f, content(idx));
      end loop;
      file_close(f);
    end;

Given that the simulator behavior isn't specified, this may fail on other simulators than Riviera-PRO and it may change in the future.

For the Python approach you can add an integer generic expected_file_size and the following code to the run script.

test006a = root / "vhdl_2019" / "test006a.txt"
expected_file_size = test006a.stat().st_size
ui.library("vhdl_2019").test_bench("tb_fileio_textio_updates").set_generic("expected_file_size", expected_file_size)

This wouldn't work when manually testing without VUnit for XSim and other unsupported simulators. What you can do is to add a default value of -1 to the generic and add VHDL code that will fail on -1 and instruct the user to manually edit the default value to the correct file size as indicated by their operating system

nickg commented 1 year ago

I had another go at this:

nickg commented 1 year ago

Any more comments on this one? I'm unable to test on Windows but it's working well for me on Linux.