-
Notifications
You must be signed in to change notification settings - Fork 79
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
Testing: sharness environment #363
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d5a3e08
Testing: sharness environment setup
0xGabi 2057d2c
Add sharness tests to help and version
kernelwhisperer 5780d49
Merge remote-tracking branch 'upstream/master' into add-sharness
0xGabi c5afe82
Merge branch 'add-sharness' of https://github.com/0xGabi/aragon-cli i…
0xGabi 855c513
Test: Add create-aragon-app and aragon init test
0xGabi 91f18f3
Test: Update create-aragon-app and init tests
0xGabi 85512fe
Add aragon ipfs
kernelwhisperer 04715f7
Fix attempt for aragon_ipfs test
kernelwhisperer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
lib/sharness/ | ||
test-results/ | ||
trash directory.*/ | ||
plugins |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
# Run tests | ||
|
||
SHELL_PATH ?= $(SHELL) | ||
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) | ||
RM ?= rm -f | ||
PROVE ?= prove | ||
LIBDIR = lib | ||
SHARNESSDIR = sharness | ||
AGGREGATE_SCRIPT ?= $(LIBDIR)/$(SHARNESSDIR)/aggregate-results.sh | ||
DEFAULT_TEST_TARGET ?= test | ||
|
||
T = $(sort $(wildcard *.t)) | ||
|
||
all: $(DEFAULT_TEST_TARGET) | ||
|
||
test: pre-clean aggregate-results-and-cleanup | ||
|
||
$(T): sharness | ||
|
||
@echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(TEST_OPTS) | ||
|
||
pre-clean: | ||
@echo "*** $@ ***" | ||
$(RM) -r test-results | ||
|
||
clean: | ||
@echo "*** $@ ***" | ||
$(RM) -r test-results | ||
|
||
aggregate-results-and-cleanup: $(T) aggregate-results clean | ||
|
||
aggregate-results: | ||
@echo "*** $@ ***" | ||
@for f in test-results/*.counts; do \ | ||
echo "$$f"; \ | ||
done | '$(SHELL_PATH_SQ)' '$(AGGREGATE_SCRIPT)' | ||
|
||
sharness: | ||
@echo "*** checking $@ ***" | ||
lib/install-sharness.sh | ||
|
||
prove: clean | ||
@echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(PROVE_OPTS) $(T) :: $(TEST_OPTS) | ||
$(MAKE) clean-prove-cache | ||
|
||
clean-prove-cache: | ||
$(RM) .prove | ||
|
||
.PHONY: all test prove $(T) pre-clean clean sharness | ||
.PHONY: aggregate-results-and-cleanup aggregate-results |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
# aragonCLI-sharness-tests | ||
|
||
AragonCLI whole tests using the [Sharness framework](https://github.com/chriscool/sharness/) | ||
|
||
## Running all the tests | ||
|
||
Just use `make` in this directory to run all the tests. | ||
|
||
## Running just one test | ||
|
||
You can run only one test script by launching it like a regular shell | ||
script: | ||
|
||
``` | ||
$ ./sharness.t | ||
``` | ||
|
||
## Command-line options | ||
|
||
The `*.t` test scripts have the following options: | ||
|
||
- `--debug`, `-d`: helps debugging | ||
- `--immediate`, `-i`: stop execution after the first failing test | ||
- `--long-tests`, `-l`: run tests marked with prereq EXPENSIVE | ||
- `--interactive-tests`: run tests marked with prereq INTERACTIVE | ||
- `--help`, `-h`: show test description | ||
- `--verbose`, `-v`: show additional debug output | ||
- `--quiet`, `-q`: show less output | ||
- `--chain-lint`/`--no-chain-lint`: check &&-chains in scripts | ||
- `--no-color`: don't color the output | ||
- `--tee`: also write output to a file | ||
- `--verbose-log`: write output to a file, but not on stdout | ||
- `--root=<dir>`: create trash directories in `<dir>` instead of current directory. | ||
|
||
## Sharness | ||
|
||
When running sharness tests from main Makefile, dependencies for sharness | ||
will be downloaded from its github repo and installed in a "lib/sharness" | ||
directory. | ||
|
||
Please do not change anything in the "lib/sharness" directory. | ||
|
||
## Writing Tests | ||
|
||
Start with the [sharness API](https://github.com/chriscool/sharness/blob/master/API.md) and then please have a look at existing tests and try to follow their example. | ||
|
||
It should be possible to put most of the code inside `test_expect_success`, | ||
or sometimes `test_expect_failure`, blocks, and to chain all the commands | ||
inside those blocks with `&&`, or `||` for diagnostic commands. | ||
|
||
### Diagnostics | ||
|
||
Make your test case output helpful for when running sharness verbosely. | ||
This means cating certain files, or running diagnostic commands. | ||
For example: | ||
|
||
``` | ||
test_expect_success ".ipfs/ has been created" ' | ||
test -d ".ipfs" && | ||
test -f ".ipfs/config" && | ||
test -d ".ipfs/datastore" && | ||
test -d ".ipfs/blocks" || | ||
test_fsh ls -al .ipfs | ||
' | ||
``` | ||
|
||
The `|| ...` is a diagnostic run when the preceding command fails. | ||
test*fsh is a shell function that echoes the args, runs the cmd, | ||
and then also fails, making sure the test case fails. (wouldnt want | ||
the diagnostic accidentally returning true and making it \_seem* like | ||
the test case succeeded!). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
#!/bin/sh | ||
|
||
test_description="Test aragon --help command" | ||
|
||
. ./lib/test-lib.sh | ||
|
||
TEXT_IT_SHOULD_INCLUDE="For more information, check out https://hack.aragon.org" | ||
|
||
test_expect_success "'aragon --help' succeeds" ' | ||
aragon --help | ||
' | ||
|
||
# test_expect_success "'aragon --help' is fast" ' | ||
# # silent the output of aragon --help by redirecting it to: /dev/null | ||
# # redirect the output of time (which goes to stderr/2 normally) to stdout/1 with: 2>&1 | ||
|
||
# OUTPUT=$(TIMEFORMAT='%lR'; time (aragon --help > /dev/null) 2>&1) && | ||
# echo "$OUTPUT" | ||
|
||
# if [[ $OUTPUT == "0m5.883s" ]]; #TODO find a way to parse this and compare it | ||
# then return 0 | ||
# else return 1 | ||
# fi | ||
# ' | ||
|
||
# pipe output to grep | ||
test_expect_success "'aragon --help' output includes the hack website (1)" ' | ||
aragon --help | grep "$TEXT_IT_SHOULD_INCLUDE" | ||
' | ||
|
||
# save output to file and use grep | ||
test_expect_success "'aragon --help' output includes the hack website (2)" ' | ||
aragon --help > actual && | ||
grep "$TEXT_IT_SHOULD_INCLUDE" actual | ||
' | ||
|
||
# save output to variable and use grep | ||
test_expect_success "'aragon --help' output includes the hack website (3)" ' | ||
OUTPUT=$(aragon --help) && | ||
if [[ $OUTPUT =~ "$TEXT_IT_SHOULD_INCLUDE" ]]; | ||
then return 0 | ||
else return 1 | ||
fi | ||
' | ||
|
||
test_done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
#!/bin/sh | ||
|
||
test_description="Test aragon init command" | ||
|
||
. ./lib/test-lib.sh | ||
|
||
APP_NAME="test-app" | ||
|
||
test_expect_success "'aragon init' succeeds" ' | ||
test_might_fail aragon init "$APP_NAME" | ||
' | ||
|
||
# test arapp.json have app name updated | ||
test_expect_success "arapp.json appName updated" ' | ||
echo 3 > result | ||
grep "\"appName\": \""$APP_NAME".*.aragonpm.eth" "$APP_NAME"/arapp.json -c > matchs | ||
test_cmp result matchs | ||
' | ||
|
||
#test if project already exists | ||
test_expect_success "project already exists" ' | ||
test_must_fail aragon init "$APP_NAME" > output.txt && | ||
grep "Project with name "$APP_NAME" already exists" output.txt | ||
' | ||
|
||
test_done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
#!/bin/sh | ||
|
||
test_description="Test aragon ipfs command" | ||
|
||
. ./lib/test-lib.sh | ||
|
||
test_launch_aragon_ipfs() { | ||
test_expect_success "'aragon ipfs' succeeds" ' | ||
aragon ipfs > out_file 2> err_file & | ||
PID=$! && | ||
echo "Process ID of the shell running this: $PID" && | ||
PGID=$(test_get_pgid_from_pid $PID) && | ||
echo "Process Group ID: $PGID" | ||
' | ||
# TODO Memory leak: tail never finishes | ||
# TODO process substitution is not portable, only works in bash | ||
test_expect_success "'aragon ipfs' says all good" ' | ||
timeout 180 grep -m 1 "running" <(tail -f out_file) | ||
' | ||
# TODO test_curl_resp_http_code "http://localhost:5001/api/v0/version" "HTTP/1.1 200 OK" | ||
# TODO test check aragon-cache "http://127.0.0.1:$API_PORT/ipfs/$HASH" "HTTP/1.1 200 OK" | ||
} | ||
|
||
test_kill_ipfs() { | ||
# -- is the default, which also means -15 or -TERM, sending the Termination signal | ||
# consider doing a graceful shutdown with -INT (-2) (interrupt - same as CTRL+C) | ||
# or using -KILL (-9) if a clean termination does not work | ||
test_expect_failure "'aragon ipfs' can be terminated" ' | ||
kill -9 -$PGID | ||
' | ||
} | ||
|
||
test_launch_aragon_ipfs | ||
|
||
test_kill_ipfs | ||
|
||
test_done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#!/bin/sh | ||
|
||
test_description="Test aragon --version command" | ||
|
||
. ./lib/test-lib.sh | ||
|
||
EXPECTED_OUTPUT="5.3.3" | ||
|
||
test_expect_success "'aragon --version' succeeds" ' | ||
aragon --version | ||
' | ||
|
||
# save outputs to files and compare | ||
test_expect_success "'aragon --version' output looks good (1)" ' | ||
aragon --version > actual && | ||
echo $EXPECTED_OUTPUT > expected && | ||
test_cmp expected actual | ||
' | ||
|
||
# save the output in a variable and compare | ||
test_expect_success "'aragon --version' output looks good (2)" ' | ||
OUTPUT=$(aragon --version) && | ||
|
||
if [[ $OUTPUT == $EXPECTED_OUTPUT ]]; | ||
then return 0 | ||
else return 1 | ||
fi | ||
' | ||
|
||
test_done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
#!/bin/sh | ||
|
||
test_description="Test create-aragon-app command" | ||
|
||
. ./lib/test-lib.sh | ||
|
||
APP_NAME="test-app" | ||
|
||
test_expect_success "'create-aragon-app' succeeds" ' | ||
test_might_fail npx create-aragon-app "$APP_NAME" | ||
' | ||
|
||
# test arapp.json have app name updated | ||
test_expect_success "arapp.json appName updated" ' | ||
echo 3 > result | ||
grep "\"appName\": \""$APP_NAME".*.aragonpm.eth" "$APP_NAME"/arapp.json -c > matchs | ||
test_cmp result matchs | ||
' | ||
|
||
#test if project already exists | ||
test_expect_success "project already exists" ' | ||
test_must_fail npx create-aragon-app "$APP_NAME" > output.txt && | ||
grep "Project with name "$APP_NAME" already exists" output.txt | ||
' | ||
|
||
test_done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
#!/bin/sh | ||
# install sharness.sh | ||
# | ||
|
||
# settings | ||
version=827ec00027125fc0ba56397beb05c15b06a67606 | ||
urlprefix=https://github.com/chriscool/sharness.git | ||
if test ! -n "$clonedir" ; then | ||
clonedir=lib | ||
fi | ||
sharnessdir=sharness | ||
|
||
if test -f "$clonedir/$sharnessdir/SHARNESS_VERSION_$version" | ||
then | ||
# There is the right version file. Great, we are done! | ||
exit 0 | ||
fi | ||
|
||
die() { | ||
echo >&2 "$@" | ||
exit 1 | ||
} | ||
|
||
checkout_version() { | ||
git checkout "$version" || die "Could not checkout '$version'" | ||
rm -f SHARNESS_VERSION_* || die "Could not remove 'SHARNESS_VERSION_*'" | ||
touch "SHARNESS_VERSION_$version" || die "Could not create 'SHARNESS_VERSION_$version'" | ||
echo "Sharness version $version is checked out!" | ||
} | ||
|
||
if test -d "$clonedir/$sharnessdir/.git" | ||
then | ||
# We need to update sharness! | ||
cd "$clonedir/$sharnessdir" || die "Could not cd into '$clonedir/$sharnessdir' directory" | ||
git fetch || die "Could not fetch to update sharness" | ||
checkout_version | ||
else | ||
# We need to clone sharness! | ||
mkdir -p "$clonedir" || die "Could not create '$clonedir' directory" | ||
cd "$clonedir" || die "Could not cd into '$clonedir' directory" | ||
|
||
git clone "$urlprefix" || die "Could not clone '$urlprefix'" | ||
cd "$sharnessdir" || die "Could not cd into '$sharnessdir' directory" | ||
checkout_version | ||
fi | ||
exit 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
|
||
SHARNESS_LIB="lib/sharness/sharness.sh" | ||
|
||
. "$SHARNESS_LIB" || { | ||
echo >&2 "Cannot source: $SHARNESS_LIB" | ||
echo >&2 "Please check Sharness installation." | ||
exit 1 | ||
} | ||
|
||
# Quote arguments for sh eval | ||
shellquote() { | ||
_space='' | ||
for _arg | ||
do | ||
# On Mac OS, sed adds a newline character. | ||
# With a printf wrapper the extra newline is removed. | ||
printf "$_space'%s'" "$(printf "%s" "$_arg" | sed -e "s/'/'\\\\''/g;")" | ||
_space=' ' | ||
done | ||
printf '\n' | ||
} | ||
|
||
# Echo the args, run the cmd, and then also fail, | ||
# making sure a test case fails. | ||
test_fsh() { | ||
echo "> $@" | ||
eval $(shellquote "$@") | ||
echo "" | ||
false | ||
} | ||
|
||
# Same as sharness' test_cmp but using test_fsh (to see the output). | ||
# We have to do it twice, so the first diff output doesn't show unless it's | ||
# broken. | ||
test_cmp() { | ||
diff -q "$@" >/dev/null || test_fsh diff -u "$@" | ||
} | ||
|
||
test_get_pgid_from_pid() { | ||
awk '{print $5}' < /proc/$1/stat | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While learning
bash
is quite fun... should we reconsider usingsharness
?A seemingly easy task like killing a background process:
aragon ipfs
, proves to be very time consuming. 👇After looking through ipfs' examples, the idea was simple:
The issue here was that
![image](https://user-images.githubusercontent.com/26041347/53158977-8c4dfd80-35cd-11e9-812a-231bc417fc4c.png)
kill $PID
would only kill the shell process that invokednode
, leavingnode
andipfs
running.The solution is using
kill -- -$PGID
which is the syntax for killing aprocess group id
. Luckily for us thePID
andPGID
are identical and retuned by$!
:Even though it works well from the terminal 🎉, with
sharness
, it proves a bit more difficult:PID
is not identical to thePGID
PID
returned by$!
is part of includes thesharness
process, so not only it kills thenode
+ipfs
processes, but also the test 😅What we could do is filter the node processes that are part of the
PGID
, but it feels like this should be easier (other tests too - not only this specific example).How about we write these tests with with
node
andava
? @0xGabi @sohkaigrep -m 1 "running" <(tail -f out_file)
vs
web3.js
withlocalhost:8545
to test that ganache is working correctly, make calls to contracts like ENS to check if they're setup correctly, etc.I'm not
100%
sure, but I think we could do withnode
anything we would withbash
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x6431346e Looking at these tests, it does seem much more maintainable if we just use node tooling rather than try and learn bash intricacies. Most people are not that well versed with shells, including those on the team, and this looks to be more of a nightmare to maintain than it would help 😅.
A few alternatives:
nixt
as a frameworkThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, make sense. @0x6431346e
ava
seems the easy choice here. @sohkai Is there a reason you chose that framework over other options?I would like us to consider
Jest
for aragonCLI cause test coverage right now is almost 0 and seems a really flexible framework to experiment with.In the less likely case we want to stick with TAP there is node-tap as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nixt
supports other test runners; it simply might make some interactions with running CLI commands easier (instead of doing it ourselves like in the first two suggestions).jest
is nice as it supports snapshots.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found out that
ava
also supports snapshot testing, so FWIW we could just useava
🤷♂️There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Brett!