Missing test coverage

[patricksurry]

More out of curiosity I ran the test suite thru c65 and looked for holes in the ROM heatmap indicating code that was never executed. Overall the test coverage is amazing!

There are a few branch cases that never get taken, and some errors and strings that aren't exercised, but beyond that I only found a few words that aren't tested:

• xt_abort_quote (this is noted as hard to test)
• ~xt_at_xy~
• ~literal_runtime (not exercising the non-inline literal case now)~
• ~xt_page~
• ~xt_spaces::_lots_of_spaces~
• ~xt_see~
• ~xt_words~
• ~xt_d_dot~
• ~xt_ud_dot~
• ~xt_disasm~
• ~xt_order~ (bug fixed)
• ~xt_block_c65_init~
• various commands within xt_ed
• xt_editor_el

I guess it'd be easy to add tests for most of these, even if just to validate that their output isn't changing unexpectedly (e.g. see and disasm)

[patricksurry]

For reference I added a doubled -gg option for c65 to skip the initial monitor entry and just start running. Then you can run tests like:

 cat tests/tester.fs `ls tests/*.fs | grep -v tester` - | c65/c65 -r taliforth-py65mon.bin -gg

type bye when the tests are done and then you can interact with the monitor. it's slightly tempermental (I think zsh is intercepting some stuff, and I needed stty sane) but works well enough. might be better to support an initial input file directly in c65.

[SamCoVT]

I agree that for see and disasm, I probably don't want to test their output, but we can run them and just record their output in the results so it will show up in a diff if there are changes (expected or not). I think words falls into that category as well. It's possible that users will add or remove words, and I don't want the test suite failing because of an extra word. We could capture the output of words and search through it for drop and bye (as both of those words should always be defined and they bracket the "comes with Tali" words). That would let us know if the dictionary got messed up and we were missing words (although I'd expect other tests to fail in that case as well).

There is a test for d. and d.r in tests/double.fs, but it relies on M*/, which Tali does not have, so it's commented out. M/ is just used for generating the initial constants, so it's possible to figure out what those are and just enter the raw constants directly, I think. I have a copy of M/ that works on Tali in: double_words.fs.

For SPACES, were you thinking of something like 257 as lots of spaces?

SEE and WORDS have a place in tests/tools.fs, but there are no tests in that file. I think just adding a simple test that runs the words and puts the output in results.txt should be enough to let us know if something changes. All the other words in that file can have a test like that as well.

I'll have to look at the ed tests. I thought they were reasonably comprehensive, but I really wasn't involved much with ed and I haven't used it beyond basic editing. I generally work in blocks instead. This test file (tests/ed.fs) is also interesting because it has a facility to capture Tali's output into a string. This might be useful for some of the above tests (like spaces) if you want to verify exactly what was printed during a command.

The editor_el word should be relatively easy to add a test for.

When words are tested, you should also update the header above the xt_xxxxx to note that it is automatically tested. Here's xt_d_dot as an example:

; ## D_DOT ( d -- ) "Print double"
; ## "d."  tested  ANS double

The word tested means it has been manually tested. Change it to auto when it is tested by the test suite. This shows up in docs/WORDLIST.md in the STATUS column. You can also scan that file to see what isn't currently automatically tested, and I agree that the testing is quite comprehensive (which has saved my butt on more than one occasion when I broke things while working on Tali's internals).

[patricksurry]

for spaces there's an explicit _lots_of_spaces code block that handles n > 255. I just did a quick rewrite that removes the special case, which is shorter and faster so I'll put that up in a bit.