-
-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix POSIX compliance issues uncovered by smoosh (Run smoosh test suite and make a report) #331
Comments
notes:
|
I love the idea of a public leaderboard of shell characteristics. I think incorporating some of modernish's 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). |
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 :) |
@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).
|
Just ran them with 0.7.pre2, got 97/157. A couple issues:
|
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:
|
Caught by smoosh test suite. Related to #331.
This one was also caught by the test suite: |
- Shell paths are now absolute - Add a bunch of flags to sh_spec.py to support smoosh - Don't import a few tests that can't be run even with 'timeout 1s'. TODO: Debug this. Running 149 cases out of 156 or so. Output: FATAL: 249 tests failed (72 osh failures) (note: 249 counts each shell failure separately.) Addresses issue #331.
- Adjust the environment - Add support for a timeout result, show in purple - Tweak summary text Addresses issue #331.
- smoosh.test.sh runs with the coreutils timeout binary - smoosh-hang.test.sh runs with the smoosh timeout shell script Also: - Add --rm-tmp to clear the test dir. - Handle stdout without trailing newline Addresses issue #331.
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.
|
Looking at 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 Other |
Also OSH disagrees with 3 shells on semantics.var.star.emptyifs.test which might be related to #707 |
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).
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.
The text was updated successfully, but these errors were encountered: