Issue getting tests to run properly

[SamCoVT]

I pulled in the string_literal PR (#123) and it appears some of the tests that capture the output have broken. I did have to change one line in the tests (for /MOD - and that is one of the ones that is failing) to select the version of see that adds spaces to try to line up the ASCII display of the data on the very right side of the DUMP. It's possible I mucked up at least one of the tests, but I didn't have to touch the others (or maybe I should have, but don't know that).

It looks like @patricksurry might have fixes for at least one of the errors in a different PR (#112). I'm guessing the output is just slightly different than expected, but I'll need to sit down and run the tests manually to see what's up. I should have time this weekend, but would welcome a quick PR to fix it if @patricksurry knows what is wrong and wants to fix it.

Summary for: core_a core_b core_c string double facility ed asm tali tools block search user cycles
Tali Forth 2 ran all tests requested
T{ capture-output see cword restore-output s"  NN 1 " search -rot 2drop -> true }T INCORRECT RESULT: T{ capture-output see cword restore-output s"  NN 1 " search -rot 2drop -> true }T ACTUAL RESULT: { 0 } ok
T{ capture-output see cword restore-output s"  NN 1 " search -rot 2drop -> true }T INCORRECT RESULT: T{ capture-output see cword restore-output s"  NN 1 " search -rot 2drop -> true }T ACTUAL RESULT: { 0 } ok
T{ capture-output see /mod restore-output see-/mod-output  compare-glob -> 0 }T INCORRECT RESULT: T{ capture-output see /mod restore-output see-/mod-output  compare-glob -> 0 }T ACTUAL RESULT: { -2 } ok
T{ capture-output see case restore-output see-case-output  compare-glob -> 0 }T INCORRECT RESULT: T{ capture-output see case restore-output see-case-output  compare-glob -> 0 }T ACTUAL RESULT: { -2 } ok
T{ capture-output see exit restore-output see-exit-output  compare-glob -> 0 }T INCORRECT RESULT: T{ capture-output see exit restore-output see-exit-output  compare-glob -> 0 }T ACTUAL RESULT: { -2 } ok

[patricksurry]

ya, i tried to fix in the other PRs. There was one 'ed' test that I think was broken where i'd left a dump when I updated before, and that was masking a real failure.

The sliteral PR added the padding to dump so that does change the expected output in a couple of tools.fs tests but those should have been in the original PR. For me all the tests are passing (including above) on master after pulling from your master-64tass, other than one for ed which shows

Return stack: 99 A0 E3 87 F9 D7 9A 80

I added the fix for that to the other open PRs

[patricksurry]

one other thought - wonder if mac vs pc line endings might be to blame? The tests are explicitly looking for \n, see the debug comment in tools.fs:

\ these tests are a little fiddly since the width of some fields can vary based on the base
\ and the starting offset.  HEX is better, but with DECIMAL in disasm it can be sensitive.
\ our simple compare-glob does a minimal match on * but is enough to identify a non-blank
\ number field delimited by whitespace.  For fixed width fields like hex bytes, it's better to
\ wildcard individual characters, e.g. ?? matches exactly two arbitrary characters.
\ To debug, add 2DUP DUMP after both restore-output and see-/mod and compare in results.txt.
T{ capture-output see /mod restore-output see-/mod-output  compare-glob -> 0 }T

[patricksurry]

hmm, on my windows box i can build c65 (with mingw32 toolchain) and taliforth-c65.bin which works fine interactively. but python talitest_c65.py just hangs for me - if I type "bye" (which is not echoed) in the window where it's hanging, it exits and results.txt contains just the c65 startup, bye and c65 exit text. it's like the subprocess.communicate() is just throwing away/ignoring the input test string and waiting for my input. super confused.

do you use anything like this workflow?

[SamCoVT]

Well, I'm on a different Linux machine (both machines used so far are Linux) and it's working fine. I'll have to double check and make sure I didn't muck up a branch or something on the other computer.

With regards to your windows adventure, are you using the Git shell? It doesn't have pty support and doesn't handle stdin properly for character by character operations (which Tali does). I have a windows machine I can test on, so now that my issue appears to be resolved, I'll check on yours.

[patricksurry]

I don't use windows enough so I don't have a good "known working" path. If you have a recipe for native windows (msys2? mingw in powershell?) i could also test and document.

[SamCoVT]

I agree that testing is broken on native windows. First of all, I had to edit the following line: C65_LOCATION = '../c65/c65.exe' to add the .exe on the end, or I got a CreateProcess error. Also, as a side note, make keeps building c65 every time, I think for the same reason (c65 vs c65.exe). A silly but workable solution would be to name the executable c65.exe on all systems.

Once that was resolved, the test started running but stalled. I typed words and bye and it quit on bye. Looking in tests/results.txt showed the output from words and bye. Notepad shows double-spaced lines and says "Macintosh (CR)" at the bottom (not your newer kind of mac that uses LF only, but the old kind that used CR only at the end of lines). Opening in a hex editor shows 0x0D 0x0D (eg. CR CR LF instead of CR LF) - that implies there is a "\r\n" somewhere that probably should just be "\n". Windows systems normally expand "\n" to "\r\n" automatically for many programming languages. I don't believe this has anything to do with your issue, however, as your biggest issue seems to be that STDOUT is redirected, but STDIN is not.

It's probably worth noting that kbhit and getch access the console directly and bypass the whole C stdin mechansim. Those functions are used on native windows only, and are likely the culprit here.

[SamCoVT]

I'm in native windows using just MinGW (the full fat one that include make and the like)

[SamCoVT]

In WSL, I get a stall at the same place, however typing bye does nothing and results.txt is unchanged (as if it never got opened for writing). Pressing CTRL-C interrupts process.communicate.

Is the process.communicate somewhat new? Perhaps it makes sense to write all input you want to a file and just redirect it to c65? Isn't that what you were doing previously?

[SamCoVT]

On checking, it looks like I downloaded just MinGW and GNU Make for windows - I don't believe I have msys2 installed.

[SamCoVT]

I had trouble compiling c65 under cygwin - it detects the OS is windows and tries to use the windows routines - we'll probably have to check specifically for cygwin. Once I manually compiled it (not including the flag for Windows), I was able to run the tests fine under cygwin.

[SamCoVT]

I'm "cleaning" c65 before trying on each Windows "platform" to try to eliminate cross-platform issues (eg. WSL vs cygwin vs MinGW). In MinGW (using Windows CMD shell), make clean doesn't work in the c65 directory (rm doesn't exist), and make in the c65 directory fails, I think because I don't have perl installed and it's trying to make the tests as well as the executable. I'd rather not have a perl dependency, so we may need to look at how to do that cross-platform without perl.

Native windows with MinGW compiled c65 definitely shows the issue (using Python 3.11.2 for Windows). talitest_c65.py does need C65_LOCATION = ../c65/c65.exe instead of C65_LOCATION = ../c65/c65

Cygwin, after manually compiling c65, works fine (using Python 3.9.16 for Cygwin) as long as talitest_c65.py has C65_LOCATION = ../c65/c65.exe

WSL works fine. I think I just needed to "clean" c65 before building it, so my previous failure here may have been using an already compiled version of c65.

Because this doesn't really pose an issue for using Tali, I don't consider this a big issue at this exact moment, but it would be nice to have this working better. We had discussed recommending using WSL for Windows users, and that seems to work in this case. I'll leave this issue open for now if you have any other findings or thoughts.

[patricksurry]

kbhit and getch access the console directly and bypass the whole C stdin mechansim

yup, that seems right. it's always waiting for console input and ignoring the redirected input on stdin. that explains all the symptoms, including the weird behavior I see if I leave the test 'hanging' and then run another instance of c65/tali interactively: the interactive one seems to miss keystrokes which makes perfect sense if it's fighting with the 'hanging' process to grab keys from the console.

I don't think that subprocess.communicate stuff is new, so maybe this hasn't worked as long as we've used kbhit? this stackoverflow answer suggests an alternative approach that I'll fool around with.

[patricksurry]

also lmk if you still see any test problems on the non-windows env

[SamCoVT]

So there does appear to be an actual issue here - It looks like it's related to the disassembler. It stops after printing out the depth check. Here is an example:

see drop 
nt: BBCA  xt: 8653 
flags: CO 0 IM 1 AN 0 NN 0 HC 1 | UF 0 ST 0 
size (decimal): CFA 3  PFA 2 

8653  20 46 D8 E8 E8                                     F...

8653   D846 jsr     1 STACK DEPTH CHECK
 ok

I'm not sure how some platforms work? I'll have to go retest on those other platforms when I get a chance, and see if I can narrow down exactly what the issue is here. Let me know if you can reproduce this issue with the latest version.

[SamCoVT]

The issue is here (for see drop):

flags: CO 0 IM 1 AN 0 NN 0 HC 1 | UF 0 ST 0
size (decimal): CFA 3  PFA 2 

It shows this word as a word that has a codefield, which DROP does not. DROP has a 0 for the flags, so it's likely a problem in the code that detects HC words in see. This results in SEE only passing along 3 bytes for disasm to display, which is why only the stack depth check shows up.

Debugging further shows it's this line in xt_see:

 lda (0, x)

which is being assembled to: A5 00 8a - that's the wrong opcode. That explains why this computer is the only one with issues. It's the assembler. I'll try updating. (currently on 1.56.2625)

[SamCoVT]

Updating to 64tass v1.59.3120 solved the issue. I'll add updating the minimum version in the docs to my to-do list.

[SamCoVT]

64tass author recommends no space for ,X type indexing to eliminate this type of thing from happening (64tass isn't the only assembler that's fussy about no spaces with ,X and ,Y) - we may want to update that single line to lda (0,x) with no space after the comma so that noone runs into this issue even when running v1.56.2625 (which is the current version that apt gives you on Ubuntu). https://sourceforge.net/p/tass64/bugs/63/

[patricksurry]

Wow that's well spotted. Wonder if there's any other place where it's written that way. I'll see if I can check with regex