nim-works / nimskull

An in development statically typed systems programming language; with sustainability at its core. We, the community of users, maintain it.
https://nim-works.github.io/nimskull/index.html
Other
276 stars 39 forks source link

testament in `sexp` mode fails on Windows #263

Closed zerbina closed 1 year ago

zerbina commented 2 years ago

Running testament on Windows with sexp output formatting fails with reMsgsDiffer due to testament treating the backslash characters in the file path as the beginning of a string escape sequence.

Example

tests\new\tfile.nim:

discard """
  nimoutFormat: sexp
  cmd: "nim c --msgFormat=sexp --filenames:canonical $file"
"""

{.hint: "hint".} #[tt.Hint
     ^ (User :str "hint") ]#

Platform: Windows Commit: 365a680bd8e7bcba0b59af40e43096dcb510d5e0

Steps to reproduce:

Actual Output

Failure: reMsgsDiffer

Expected inline Hint annotation at tests\new\tfile.nim(6, 6):

- (User :location ("tests\new\tfile.nim" 6 6) :severity Hint :str "hint")

Given:

+ (User :location ("tests
ew      file.nim" 6 6) :reportFrom ("pragmas.nim" 1540 20) :reportInst ("pragmas.nim" 1540 20) :severity Hint :str "hint")

  :location[0] expected "tests\\new\\tfile.nim", but got "tests\u000Aew\u0009file.nim" (["tests\\new\\tfile.nim" -> "tests\u000Aew\u0009file.nim"])

Expected Output

Test success

canelhasmateus commented 2 years ago

Is this still a issue? I was trying it right now, and have gotten the following error:

C:\Users\Mateus\Desktop\workspace\nimskull\testament\testament.exe --nim:bin\nim.exe r tests/new/tfile.nim
FAIL: tests/new/tfile.nim c                                        ( 0.00 sec)
Test "tests\new\tfile.nim" in category "new"
Failure: reInvalidSpec
Expected:

Gotten:
invalid key for test spec: nimoutFormat

warning: LF will be replaced by CRLF in C:\Users\Mateus\AppData\Local\Temp\diffStrings_b_0u3OEuOB.
The file will have its original line endings in your working directory
diff --git "a/C:\\Users\\Mateus\\AppData\\Local\\Temp\\diffStrings_a_X4mxLfvY" "b/C:\\Users\\Mateus\\AppData\\Local\\Temp\\diffStrings_b_0u3OEuOB"
index e69de29bb..86f68d496 100644
--- "a/C:\\Users\\Mateus\\AppData\\Local\\Temp\\diffStrings_a_X4mxLfvY"
+++ "b/C:\\Users\\Mateus\\AppData\\Local\\Temp\\diffStrings_b_0u3OEuOB"
@@ -0,0 +1 @@
+invalid key for test spec: nimoutFormat

FAILURE! total: 1 passed: 0 skipped: 0 failed: 1
tests failed
zerbina commented 2 years ago

Yep, I just tried it again. It still fails due to the same issue.

It's strange that testament doesn't recognize the nimoutFormat key for you however

canelhasmateus commented 2 years ago

i tried checking out to the exact commit pointed by you. Now i'm getting the error:

FAIL: tests/new/tfile.nim c                                        ( 0.01 sec)
Test "tests\new\tfile.nim" in category "new"
Failure: reNimcCrash
debugInfo:  compiler exit code was 0 but some Error's were found.
$ C:\Users\Mateus\Desktop\workspace\nimskull\bin\nim.exe c --msgFormat=sexp --filenames:canonical tests\new\tfile.nim
command line(1, 2) Error: invalid command line option: '--msgFormat'

FAILURE! total: 1 passed: 0 skipped: 0 failed: 1
tests failed

Mind that, before that, i had to exclude the `./build/csources/.git folder : It seems each time i switch branches, i face a permission error:

PS C:\Users\Mateus\Desktop\workspace\nimskull> python koch.py test r tests/new/tfile.nim
Building koch.nim
Bootstrap compiler C:\Users\Mateus\Desktop\workspace\nimskull\build\csources\bin\nim-c181c28ae63be7c40cd316eb6744a01561617b6a.exe not found, building
Bootstrap source URL (https://github.com/nim-lang/csources_v1.git) differs from configuration (https://github.com/nim-works/csources_v1.git)
Removing bootstrap source at C:\Users\Mateus\Desktop\workspace\nimskull\build\csources and refetch
Traceback (most recent call last):
  File "C:\Users\Mateus\Desktop\workspace\nimskull\koch.py", line 278, in <module>
    main()
  File "C:\Users\Mateus\Desktop\workspace\nimskull\koch.py", line 253, in main
    bootstrap.run(
  File "C:\Users\Mateus\Desktop\workspace\nimskull\koch.py", line 226, in run
    self.build()
  File "C:\Users\Mateus\Desktop\workspace\nimskull\koch.py", line 183, in build
    self.fetch()
  File "C:\Users\Mateus\Desktop\workspace\nimskull\koch.py", line 152, in fetch
    shutil.rmtree(Bootstrap.Source)
  File "C:\Python310\lib\shutil.py", line 747, in rmtree
    return _rmtree_unsafe(path, onerror)
  File "C:\Python310\lib\shutil.py", line 612, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Python310\lib\shutil.py", line 612, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Python310\lib\shutil.py", line 612, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Python310\lib\shutil.py", line 617, in _rmtree_unsafe
    onerror(os.unlink, fullname, sys.exc_info())
  File "C:\Python310\lib\shutil.py", line 615, in _rmtree_unsafe
    os.unlink(fullname)
PermissionError: [WinError 5] Access is denied: 'C:\\Users\\Mateus\\Desktop\\workspace\\nimskull\\build\\csources\\.git\\objects\\pack\\pack-5ff9669c9803c82849d812450dface00946807a2.idx'
PS C:\Users\Mateus\Desktop\workspace\nimskull> 

@alaviss Could you help me here?

canelhasmateus commented 2 years ago

after re-cloning the project and re-building the compiler, i finally get the same error

C:\Users\Mateus\Desktop\workspace\nimskull\testament\testament.exe --nim:bin\nim.exe r tests/new/tfile.nim
FAIL: tests/new/tfile.nim c                                        ( 1.56 sec)
Test "tests\new\tfile.nim" in category "new"
Failure: reMsgsDiffer

Expected inline Hint annotation at tests\new\tfile.nim(6, 6):

- (User :location ("tests\new\tfile.nim" 6 6) :severity Hint :str "hint")

Given:

+ (User :location ("tests
ew      file.nim" 6 6) :reportFrom ("pragmas.nim" 1532 20) :reportInst ("pragmas.nim" 1532 20) :severity Hint :str "hint")

  :location[0] expected "tests\\new\\tfile.nim", but got "tests\u000Aew\u0009file.nim" (["tests\\new\\tfile.nim" -> "tests\u000Aew\u0009file.nim"])
FAILURE! total: 1 passed: 0 skipped: 0 failed: 1
tests failed
alaviss commented 2 years ago

@canelhasmateus In this case the workaround is to remove build/csources folder manually. I guess this might be a python issue (also deleting a folder on Windows is very hard...)

canelhasmateus commented 2 years ago

ok, after a bit of sleuthing , it seems i can confirm what is happening.

By adding

result.nimout = """(User :severity Hint :str "hint" :location ("tests\\new\\tfile.nim" 6 6) :reportInst ("pragmas.nim" 1532 20) :reportFrom ("pragmas.nim" 1532 20))
(SuccessX :severity Hint :buildParams (:project "tfile" :output "tfile.exe" :linesCompiled 0 :mem 41496576 :isMaxMem t :sec 0.33792710304260254 :isCompilation t :threads nil :buildMode "debug" :optimize "debug" :gc "refc") :location nil :reportInst ("msgs.nim" 766 14))
"""

before line 394 of testament.nim , we can cause the test to pass.

Note that this value is what the result would usually have, but i have manually scaped the "\n" and "\t" pertaining to the file name. What is going on is indeed an problem in regards to backslash scaping.

Is there any way we can deal with this here, @haxscramper ?

canelhasmateus commented 2 years ago

as pointed by hax in the matrix channel, the issue might lie in the sexp parser

The lazy approach - Changing line 148 to add(my.ad , "\\n") which essentially comments the line - makes this particular test pass, in detriment of the remaining test suite.


As suggested , creating a "roundabout test" seems to be a wise decision - That is:

Create report, convert to S-expression, print to the stdout, read it from there, parse again and compare with original report

I'll possibly look into that in the following days.

If not, however - @haxscramper / @alaviss / @saem / @zerbina: Do you think there is enough information here to tag it as a easy / good first issue? If not, i'd be willing to write a more thorough description.

saem commented 2 years ago

That looks good, I think some straightforward unit tests might capture much of this. What you're suggesting sounds reasonable, however.

haxscramper commented 2 years ago

I'm not sure if this is "simple" or the test is actually going to catch anything, it was just one of several suggestions I made.

There might be something wrong with the message unparsing in the first place. I haven't seen this in a while, and windows-specific problem would be just a guess in any case

haxscramper commented 2 years ago

After a bit of thinking - adding test is relatively "easy", but the whole issue is not about the test, and unless we can give (something close to) "in files X, Y, Z do the following changes ..." or "replace one type of usages of a type with another" (and so on) I wouldn't consider issue easy

At least that's what my logic behind label assignment was.