Skip to content
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

spec tests should run in a known environment #42

Closed
andychu opened this issue Sep 26, 2017 · 10 comments
Closed

spec tests should run in a known environment #42

andychu opened this issue Sep 26, 2017 · 10 comments

Comments

@andychu
Copy link
Contributor

andychu commented Sep 26, 2017

There have been some issues with assuming /usr/bin/time, /usr/bin/python, /bin/bash, etc. This seems to be the first issue everybody runs into when developing Oil.

The tests also depend on gawk I think. And the shells: busybox, dash, bash, ash, etc.

Right now I think the best solution is an Alpine Linux chroot, because it's small and easy to set up. Should be runnable on any distro quite easily.

FYI @lheckemann

Also see issue #12

@andychu
Copy link
Contributor Author

andychu commented Jun 4, 2018

14 problems on Ubuntu 17 due to bash/mksh/etc. version mismatches :-(

$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=17.10
DISTRIB_CODENAME=artful
DISTRIB_DESCRIPTION="Ubuntu 17.10"
'_tmp/web' -> '/home/andy/git/oilshell/oil/web'                                          
alias failed with status 1                  
array failed with status 1                  
assoc failed with status 1                  
blog2 failed with status 1                  
builtin-io failed with status 1             
builtins failed with status 1               
builtin-test failed with status 1           
glob failed with status 1                   
sh-options failed with status 1             
var-op-other failed with status 1           
var-op-strip failed with status 1           
var-ref failed with status 1                
var-sub failed with status 1                
xtrace failed with status 1                 

*** 14 tests FAILED 

@andychu
Copy link
Contributor Author

andychu commented Jun 4, 2018

Labelling this help wanted because maybe someone can write a shell script to make a chroot image?

I can provide more color on this.

@andychu
Copy link
Contributor Author

andychu commented Jun 8, 2018

@Yorwba Let's continue the discussion here.

I said Alpine might be harder and Ubuntu is probably easier. The reason I think that is:

  1. The tests are already written against Ubuntu
  2. Alpine is busybox-based while Ubuntu is coreutils-based.

So potentially every test has to be tweaked. But maybe this is not as big a deal as I thought.

Intermediate task (which I may pick up): Provide an alias to run a single spec test in a chroot. Then we can migrate it test-by-test.


You know I think the main issue is: what shells do we test against?

  1. Binary Ubuntu packages (which we do now essentially)
  2. Binary Alpine packages
  3. Compile shells from source on Ubuntu or Alpine

I was thinking #3 because it gives us the most control over what shell version. Although honestly I think "whatever shell version the latest LTS version of Ubuntu uses" is not bad.

I was thinking "the latest version of every shell", but in theory that is more work (though it shouldn't be THAT much work.)

I have to go back and remember how debootstrap works. If I can debootstrap Ubuntu 18.04 from my 16.04 machine, that would be nice. But I think Ubuntu doesn't necessarily do that. In other words, I don't want the version of shells we test against to be tied to "did Andy upgrade his personal machine, or does he have access to an 18.04 machine?" Right now I do not.

Alpine is nice because they provide a small chroot image to download. And it seems to run fine on Ubuntu. So there's less possibility of "hidden" dependencies between guest and host.

@andychu
Copy link
Contributor Author

andychu commented Jun 8, 2018

Thinking aloud here, another reason I have put this is off is because Ubuntu doesn't provide a nice root file system download like Alpine:

https://alpinelinux.org/downloads/

On Ubuntu you have to run debootstrap yourself and do some other nontrivial setup steps, as far as I remember.

I also have to check out what versions of dash/mksh/zsh/busybox Alpine has, if any.

@Yorwba
Copy link
Contributor

Yorwba commented Jun 9, 2018

It seems like using Alpine's edge branch should keep you pretty close to "the latest version of every shell".

E.g. the repository currently has bash 4.4.19, which was packaged the day after the upstream release. The highest patchlevel would be 4.4.23, but judging by the upload date in GNU's FTP, those patches are barely over a week old.

Looking through the packages for the other shells, the delay never seems to be more than a few weeks.

@andychu
Copy link
Contributor Author

andychu commented Jun 13, 2018

Small update on this:

  • I didn't want every developer to have to run commands as root, which chroot requires. So I investigated "rootless containers" with runc. I got a "hello world" to work, although it required more wrestling with Go's build system than I liked.

  • Along the way I realized that for the spec tests, the only thing you really care about is having dash/bash/mksh/zsh in $PATH, as well as coreutils/busybox in $PATH. So actually we should be able to have reproducible test results on a reasonable Linux machine without any kind of chroot or container. The test driver already appends to $PATH, so we could make it so it resets $PATH completely.

The downside is that this method might require everyone to build 4 or 5 shells from scratch. Or maybe I can build some "portable" binaries. I think I read about a blog post about that, which basically amounted to using an old libc (?)

https://www.cyphar.com/blog/post/20160627-rootless-containers-with-runc

andychu pushed a commit that referenced this issue Jul 4, 2018
Right now they are the versions that come with Ubuntu 16.04.

test/spec.sh: If possible, use these hermetic shell binaries for spec
tests.

Addresses issue #42.

I'm not sure why, but I had to adjust two tests.

glob.test.sh:
  Upstream dash 0.5.8 also has the same bug as mksh.

  I'm not sure why the Ubuntu package, which is marked 0.5.8, doesn't
  have it.  Maybe they ported a patch over?

quote.test.sh:
  hermetic ash doesn't have this bug for some reason.

Also: the regex test is currently failing for ZSH because we're not
building with regex support.  To be fixed.
@andychu
Copy link
Contributor Author

andychu commented Aug 5, 2018

@granttrec found out that while the shell binaries are now isolated, the environment isn't! The unicode tests depend on certain environment variables.

I honestly forget all the details, but I know that both libc and I think bash itself respect a bunch of locale environment variables.

We could probably just set the environment variables in test/spec-runner.sh itself ?? Not sure.

@ghost
Copy link

ghost commented Aug 6, 2018

@andychu setting the environment variables will work, however we will have to check for a correct locale on the system first, then set LC_ALL to the locale of choice, I assume en_US.UTF-8; en_CA.UTF-8 worked for me.

@andychu
Copy link
Contributor Author

andychu commented Aug 6, 2018

OK, I'll leave it up to you if you want to submit a patch. If someone else runs into then it may become higher priority. If you do, please mention the tests that depend on the locale in the comments, and point to any relevant libc or bash documentation.

andychu pushed a commit that referenced this issue Aug 18, 2019
In addition to $REPO_ROOT/spec/bin, we only need /usr/bin and /bin to
run!

This was motivated by a machine that somehow had '::' in its $PATH,
which made spec/builtin-eval-source fail because the '.' would respect
the current directory.

Addresses issue #42.
@andychu
Copy link
Contributor Author

andychu commented Mar 20, 2020

Done with "toil" on Travis CI!

https://github.com/oilshell/oil/tree/master/services

@andychu andychu closed this as completed Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants