Conversation
Thanks for this pull request, @chgibb! As @nathansobo mentioned on #22, we are in the process of revising the architecture of xray and this may affect the way things get tested. We'll make sure to take a look at this once #46 lands on master. Thanks! |
Now that we have merged #46, are you interested in adjusting this PR to the new reality? Now we should be able to run all of our tests without Electron. This makes @maxbrunsfeld and I think that it would be better to target Travis's Rust environment. Then we should be able to run Thanks! |
Definitely @nathansobo ! |
@chgibb I think we'll eventually want to add integration tests that touch Electron, but for now we don't have any and are getting by reasonably well with Enzyme. Since all of the real logic is in the server, I think the risk is lower than it might be for a more complex Electron app. But eventually we'll probably need some Electron-specific tests. |
Update Fork
Update Branch
I changed the script to use Travis' default rust config which calls |
This looks good. Are you up for adding a script that also runs |
Threw node and |
Can this be done without sudo? I think Travis’s container infrastructure has lower wait times. The bash script is a good idea. I’d call it ‘script/test’. |
Note that you can run most |
Split off into script and removed |
npm test | ||
if [ $? != 0 ]; then | ||
exit 1 | ||
fi |
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.
I think you could avoid a lot of these if
statements by adding set -e
to the top of the script.
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.
set -e
introduces a fair few hidden footguns by aborting after any non-zero exit code. If I remember correctly, there's also some differing behaviour between set -e
in GNU Bash, MSys2 and Cygwin. I would argue against it, and personally prefer the more manual error handling. However, I can change it.
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.
👍 No nevermind, this seems fine.
Thanks @chgibb! |
Initial config for Ubuntu on Travis. At the moment this is fairly bare bones as it only builds xray and does not run tests. See log. There were some issues with clang and rustup that required some fiddling and the commit history reflects that.
#22