[RFC] hooks: add support for .test files and add some initial tests
#110
Conversation
The current hooks do not have a good testing story. To fix this this PR adds support for "hook/foo.test" files that will be run at the end of the prime phase during the snap build. It also adds two initial tests and converts the existing ABI test. If this looks reasonable we can write tests for the remaining hooks to ensure we do not regress again.
mvo5
changed the title
.test files and add some initial tests.test files and add some initial tests
snapcraft.yaml
Outdated
| @@ -26,3 +26,5 @@ parts: | |||
| unset LD_LIBRARY_PATH; | |||
| export PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin"; | |||
| snapcraftctl prime | |||
| # ensure buildin tests are run | |||
There was a problem hiding this comment.
| # ensure buildin tests are run | |
| # ensure built-in tests are run |
| fi | ||
|
|
||
| if [ -s /etc/machine-id ]; then | ||
| echo "/etc/machine-id must exit but must be empty" |
There was a problem hiding this comment.
| echo "/etc/machine-id must exit but must be empty" | |
| echo "/etc/machine-id exists but must be empty" |
| fi | ||
|
|
||
| if [ ! -e /etc/machine-id ]; then | ||
| echo "/etc/machine-id must exit" |
There was a problem hiding this comment.
| echo "/etc/machine-id must exit" | |
| echo "/etc/machine-id must exist" |
README.md
Outdated
| are run inside the base image filesystem. | ||
|
|
||
| Each hook should have a matching `.test` file that validates that the | ||
| `.chroot` file works as expected. Those `.test` scripts will be run |
There was a problem hiding this comment.
it is not very clear from this description that these tests will be run after building with snapcraft, if I understand the travis.yml.
Also I wonder whether "sanity" or "check" would be a better name for them and this process.
There was a problem hiding this comment.
Thanks! I updated the documentation now. As for test vs check vs sanity I have no super strong opinion. Those are not tests in the traditional sense so a rename does make sense. I feel the upside of using tests is that people who write code may look for "tests" when wanting to write tests more readily than looking for check or sanity.
There was a problem hiding this comment.
Ok, this looks fine (besides those small typos as highlighted by zyga).
Personally (that's just a preference though) I don't like the idea of having both the hooks and the tests in the same directory. It might get a bit confusing at first glance, since looking at the hooks/ directory you'll see files that are not actual hooks. What I would prefer personally is moving those tests to a separate hook-tests/ or similar directory, still following the same layout for running them as here.
But that's just my 5 cents.
|
@sil2100 Thank you! I moved the test files out now, the original "design" was inspired by go (which mixes source and test in a very similar way) but as foundations own this I'm fine accommodating your preferences :) |
mvo5
force-pushed
the
more-tests
branch
2 times, most recently
from
6ff2911
to
f786c48
Compare
Only run hook tests when root otherwise the chroot to the prime dir will not work.
The current hooks do not have a good testing story. To fix this
this PR adds support for "hook/foo.test" files that will be run
at the end of the prime phase during the snap build.
It also adds two initial tests and converts the existing ABI test.
If this looks reasonable we can write tests for the remaining hooks
to ensure we do not regress again.