-
Notifications
You must be signed in to change notification settings - Fork 38
Implemented NodeJS compatibility test. #128
Implemented NodeJS compatibility test. #128
Conversation
I believe #127 should be merged first. |
Can the checking scripts be moved to a new folder? Perhaps |
Addressed the above mentioned issue, the last one from #111 and I also took the liberty of guarding against a false-negative scenario. Ready for review :) |
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.
Great stuff, a few indentation issues but otherwise ready to go!
|
||
# Run the tests on all available versions | ||
for version in ${node_versions}; do | ||
echo "Testing node-${version}!"; |
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.
Can this for block be indented?
source ~/.envirius/nv | ||
|
||
if [ $? -ne 0 ]; then | ||
echo "Unable to install envirius!" |
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.
Indent
|
||
# Dump available versions to file | ||
for version in ${node_versions}; do | ||
echo ${version} >> nodes_available.log |
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.
Indent
|
||
# Cache NodeJS environments | ||
for version in ${node_versions}; do | ||
echo "Adding NodeJS ${version} into environment cache!" && \ |
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.
Indent
echo "***Started test-run for ${version}***"; | ||
cat "test_node_${version}.log"; | ||
echo "***Finished test-run for ${version}***"; | ||
done |
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.
Nice touch
@@ -28,6 +28,21 @@ jobs: | |||
- run: | |||
name: test | |||
command: npm test | |||
- run: |
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.
22 seconds, awesome!
I think everything should be fine now ;) |
# Quick check | ||
if [ "$1" == "--quick" ]; then | ||
lts_versions=$( | ||
for lts in 4 6 8 9; do |
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.
lts_version=$(
for lts in 4....
done
)
I'm really bad with indentation :)) Hopefully it's ok now. |
Enhances #111.