search

oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.85k stars 158 forks source link

Fix POSIX compliance issues uncovered by smoosh (Run smoosh test suite and make a report) #331

Open andychu opened 5 years ago

andychu commented 5 years ago

From Zulip, the way to run them against OSH is:

TEST_SHELL=osh TEST_DEBUG=1 make

I did this and got 89/153 on OSH 0.6.pre21, while @mgree got 86/153 (probably due to some environment differences).

shell_tests.sh: shell/sh.ps1.override: expected STDERR shell/sh.ps1.override.err differs from logged STDERR /home/andy/git/languages/smoosh/tests/log/2019-06-10_16:31/shell/sh.ps1.override.err
shell_tests.sh: shell/sh.ps1.override failed
shell_tests.sh: shell/sh.set.ifs passed
shell_tests.sh: 89/153 tests passed
Makefile:11: recipe for target 'shell' failed
make: *** [shell] Error 1

The format that is very compatible with our spec tests (code snippet, stdout, stderr, exit status).

https://github.com/mgree/smoosh/tree/master/tests

It should be easy to adapt test/sh_spec.py to run them and produce an HTML report.

Conversely, I will also try running smoosh in our spec tests.

@mgree FYI I just read through the repo and test harness and realized that these tests should be pretty easy to adapt (without any reformatting).

Then we will get a report like this: http://www.oilshell.org/git-branch/dev/andy-14/6da1da1a/spec/

which I like because you can scan across a row to see where shells agree and disagree.

I'm not sure when I'll get to this but I think it will be useful to run on a regular basis.

As mentioned I would also accept PRs to add annotations to the Oil test suite for smoosh. But I think it makes sense to try to import your 153 test cases as well, and doesn't look too difficult.

andychu commented 5 years ago

notes:

mgree commented 5 years ago

I love the idea of a public leaderboard of shell characteristics. I think incorporating some of modernish's FTL_ and BUG_ and QRK_ tests along with modernish --test would be a good move, too.

Please incorporate whatever tests you like! They're under MIT license; a URL and a copy of my license somewhere near those tests would be great.

What do you need from me in order to have smoosh show up on that leaderboard? I know the build process is complicated, but I could probably package up something nicer for release (e.g., drop the Lem dependency).

andychu commented 5 years ago

I think the first cut can just read the testdata data as is from your repo, but we'll see. The format is very similar to what I use.

I didn't try building smoosh yet, but I'll let you know if I have any issues.

If we find another bug or two in OSH I'll consider it worth it :)

andychu commented 5 years ago

@mgree Also I remembered I linked to a ksh test suite here, which was forked for mksh:

http://www.oilshell.org/blog/2017/06/22.html

According to the README there are some cases that are considered POSIX. Not sure exactly how they are labeled.

I never tried running these tests, probably because OSH was too immature back then. They also have a very similar format, although it requires some parsing (done in an 1000-line Perl script).

name: xxx-set-option-1
description:
    Check option parsing in set
stdin:
    set -vsA foo -- A 1 3 2
    echo ${foo[*]}
expected-stderr:
    echo ${foo[*]}
expected-stdout:
1 2 3 A
andychu commented 5 years ago

Just ran them with 0.7.pre2, got 97/157.

A couple issues:

+  shopt -s globstar  # long overdue, try out wc -l **/*.py
+  ^~~~~
+/home/andy/git/dotfiles//interactive.bash:23: 'shopt' got invalid option 'globstar'
+Terminated
shell_tests.sh: shell/sh.ps1.override: expected STDERR shell/sh.ps1.override.err differs from logged STDERR /home/andy/git/languages/smoosh/tests/log/2019-07-26_20:53/shell/sh.ps1.override.err
shell_tests.sh: shell/sh.ps1.override failed
shell_tests.sh: shell/sh.set.ifs passed
shell_tests.sh: 97/157 tests passed
Makefile:11: recipe for target 'shell' failed
make: *** [shell] Error 1
andychu commented 5 years ago

Known differences - https://www.oilshell.org/release/0.7.pre2/doc/known-differences.html

Special builtin rule:

Real bugs:

Fixed:

Minor:

Unimplemented:

Probably don't care:

andychu commented 5 years ago

This one was also caught by the test suite:

https://github.com/oilshell/oil/commit/dd426782582378a8959e07994bb583945447e008

andychu commented 5 years ago

Latest results:

http://www.oilshell.org/git-branch/dev/andy-14/b15b66a8/spec/smoosh.html

http://www.oilshell.org/git-branch/dev/andy-14/b15b66a8/spec/smoosh-hang.html (tests that seem to require SIGKILL)

After fixing a bunch of issues, this seems pretty accurate.

andychu commented 2 years ago

Looking at trap tests now

https://www.oilshell.org/release/0.9.7/test/spec.wwz/survey/smoosh.html

It also disagrees on case 53, but that's intended since OSH is stricter about handling invalid KILL and 9 signals.

builtin.trap.kill.undef.test

Other trap cases don't seem to have that much consensus from shells

andychu commented 2 years ago

Also OSH disagrees with 3 shells on

semantics.var.star.emptyifs.test

which might be related to #707