Make the integration test fully-automatic. #81
Conversation
This entails two major changes: - In the Cloud Functions, keep track of which tests pass, and return 200 "PASS" when all tests pass, or 500 "FAIL - [debug-info]" when they don't. - On the client-side, trigger the HTTP function and interpret the return code as a pass or fail. Also tries to improve the readability of 'run_tests.sh' by indenting appropriately, and improve the usability by more clearly indicating whether tests have passed or failed.
rjhuijsman
force-pushed
the
rjh.auto-integration-test
branch
from
e0d95ab to
39caf7c
Compare
| export * from './pubsub-tests'; | ||
| export * from './database-tests'; | ||
| export * from './auth-tests'; | ||
| let numTests = 4; // Make sure to increase this as you add tests! |
There was a problem hiding this comment.
instead of hardcoding this, I'd recommend naming the array of promises, then setting numTests to the length of that array. Then the next developer doesn't have to change this number if they add tests.
There was a problem hiding this comment.
The tests are not actually not quite the same as the number of promises, but they are the same as the number of exported functions - so I've gone off that. :)
integration_test/run_tests.sh
Outdated
| echo "##### Deploying empty index.js to project..." && | ||
| ./functions/node_modules/.bin/tsc -p functions/ && # Make sure the functions/lib directory actually exists. | ||
| echo "" > functions/lib/index.js && | ||
| firebase deploy --debug 2> /dev/null && |
There was a problem hiding this comment.
should probably also put --only functions here
| echo "##### Removing all functions" && | ||
| echo "" > functions/lib/index.js && | ||
| firebase deploy --debug 2> /dev/null && | ||
| rm functions/firebase-functions.tgz && |
There was a problem hiding this comment.
should also add a line to remove firebase-debug.log
|
Biggest piece of feedback is that when something fails, it just prints out XXXXX Something failed XXXXXXX It should at the minimum print out the URL for the database so the user can go check what's wrong. (PS. you can easily simulate a failure by changing one of the promises in functions/index.ts to RSVP.reject())
|
- In index.ts, automatically compute the number of tests, rather than hardcoding it. - In index.ts, return a useful error message if any promise fails, not just if tests fail. - In run_tests.sh, always deploy only functions. - In run_tests.sh, clean up firebase-debug.log.
|
Good catch that that type of failure got less logs than they should - I've rejiggered the promises in index.ts so that those failures are also caught and logged with error-500. |
laurenzlong
left a comment
laurenzlong
left a comment
There was a problem hiding this comment.
Just one quick fix (add a space), then it's good to merge.
integration_test/run_tests.sh
Outdated
| firebase deploy --debug 2> /dev/null && | ||
| echo && | ||
| firebase deploy --only functions --debug 2> /dev/null && | ||
| echo && |
There was a problem hiding this comment.
I think you're missing one space here.
| // On test completion, redirect the user to the Firebase RTDB dashboard, to | ||
| // see the results stored there. | ||
| resp.redirect(`https://${process.env.GCLOUD_PROJECT}.firebaseio.com/testRuns/${testId}`); | ||
| // On test completion, check that all tests pass and reply "PASS", or provide further details. |
There was a problem hiding this comment.
Eventually we may want to break this into client + server. The HTTP function could return the test ID and then a client would poll for the ID. This would help us avoid stricter cutoffs in maximum HTTP request duration.
| ref.on('child_added', (snapshot) => { | ||
| testsExecuted += 1; | ||
| if (!snapshot.val().passed) { | ||
| reject(new Error(`test ${snapshot.key} failed; see database for details.`)); |
| }); | ||
| }).then(() => { | ||
| ref.off(); // No more need to listen. | ||
| return Promise.resolve(); |
| resp.status(200).send('PASS'); | ||
| }).catch(err => { | ||
| console.log(`Some tests failed: ${err}`); | ||
| resp.status(500).send(`FAIL - details at https://${process.env.GCLOUD_PROJECT}.firebaseio.com/testRuns/${testId}`); |
There was a problem hiding this comment.
Nits:
- the GCLOUD_PROJECT is not necessarily the subdomain to firebaseio.com; you can use functions.config().firebase.databaseUrl.
- I think the URL has to end in .json if you're linking to a database URL.
- You might actually want to consider a pantheon so it will try to load the database using the developer credentials rather than unauthenticated credentials.
3 participants
Make the integration test fully-automatic. This entails two major changes:
Also tries to improve the readability of 'run_tests.sh' by indenting appropriately, and tries to improve its usability by more clearly indicating whether the tests have passed or failed.