Related Linear issue here. This is part of a set of diffs that will allow ShellCheck to be added to the CI. See inline comments for specific details of the ShellCheck error/warning output.
Details
Details
I went to @karol's initial diff where this code was introduced (D4216) and ran the Test Plan he noted there. Specifically, I ran:
cd services yarn run-integration-tests backup yarn run-integration-tests blob yarn run-integration-tests all
However, all these failed.
I also checked out a new branch off origin/master and the same commands failed in the same fashion. So it doesn't have to do with the code in this diff, this diff is purely for ShellCheck purposes. But we should definitely figure out what's going wrong there.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
In the description of this diff you say
However, all these failed.
Does that mean that all of these failed
yarn run-integration-tests backup yarn run-integration-tests blob yarn run-integration-tests all
or just this
yarn run-integration-tests all
?
This is a bit confusing, please clarify.
Also, if you claim the tests failed, it would be good to attach the information on what exactly went wrong. If only the all option failed, maybe they failed on the tunnelbroker (which now just throws unimplemented error)?
The diff itself looks ok.
Comment Actions
Next, I ran:
Finally, I just ran:
But this is because of line 29 of this diff. Before, $SERVICES was double quoted on line 29, so the for loop was only executing once and the argument to run_integration_test was the string "identity tunnelbroker backup blob". You can see this in the output for the above screenshot, where the echo command on line 11 of run_integration_test prints $1:
So the words were not split correctly because of the double quotes. So I went and ran yarn run-integration-tests all on master (with the changes in this diff that do split SERVICES correctly), and it fails, for the reason you stated:
As you can see, it's failing on the tunnelbroker "not implemented" error.
@karol, thanks for the response! Sorry, that is confusing. Let me clarify below and attach screenshots/output of the failures.
Here's what I did to run the yarn run-integration-tests commands on origin/master:
git checkout -b integration-tests origin/master cd services
Then, I ran:
yarn run-integration-tests backup
and this was the output (view gist here):
Next, I ran:
yarn run-integration-tests blob
with the following output (view gist here:
Finally, I just ran:
yarn run-integration-tests all
which actually passes all the tests (on origin/master)!
But this is because of line 29 of this diff. Before, $SERVICES was double quoted on line 29, so the for loop was only executing once and the argument to run_integration_test was the string "identity tunnelbroker backup blob". You can see this in the output for the above screenshot, where the echo command on line 11 of run_integration_test prints $1:
So the words were not split correctly because of the double quotes. So I went and ran yarn run-integration-tests all on master (with the changes in this diff that do split SERVICES correctly), and it fails, for the reason you stated:
maybe they failed on the tunnelbroker (which now just throws unimplemented error)
This is the output (view gist here):
As you can see, it's failing on the tunnelbroker "not implemented" error.
But yarn run-integration-tests backup and yarn run-integration-tests blob are still failing. And additionally, yarn run-integration-tests all is currently failing because of tunnelbroker, but the order of services in SERVICES is "identity tunnelbroker backup blob". So if it's failing on tunnelbroker, backup and blob aren't being run yet since the command is failing early. Either way, I don't think this diff has to do with why these tests are failing, so I will land it.