-
Notifications
You must be signed in to change notification settings - Fork 5
Description
if blocks
When there are steps that run when when a particular test condition is true and others that run when it's false, the relationship between these is lost because the if key is repeated on each step. It's hard to see that these are two possible branches, which I think would make it hard to go through and pick out the dead branch once the flag's TTL goes out of date. One possible solution: permit another if syntax where a step can have just these keys: if (a command to execute), then (a list of steps to execute if the command succeeded), and optionally else (a list of steps to execute if the command failed). For example:
- name: Check whether tooling supports --only-dependencies and --only-build flags
if: ssh $(<ip) test -f cobro/.flags/surfly_test_flags
then:
- name: Build test depenencies
run: ...
- name: Run tests
run: ...
else:
- name: Build and test dependencies
run: ...- + visibility
Exit code consequences
When using an ssh command as an if command, it's possible that the ssh command itself fails due to configuration issues, network problems, or a crashed container. Because wake is using the exit code of the test command to determine whether to run the associated step, it wouldn't be able to tell the difference between this and a test that returns false. This could cause code from neither branch to execute. A safer way to test file presence could be to check that the stdout of a particular command is equal to some string. These tests could get long and ugly though, so an even better solution might be to write remote checks into wake directly:
- if:
- file_exists: cobro/.flags/surfly_test_flags
- ssh_cmd: "ssh $(<ip)"This way, wake could have its own logic for running the file existence checks, where it has a more powerful language (golang) which could more easily tell the difference between an ssh failure and a check that returns false. This would keep the jobfiles as simple as possible while still avoiding the issue of ssh failures.
Things to consider:
- it is good to have
ifas universal as possible for flexibility - simple syntax will allow people to use it without learning wake
Taken from https://github.com/surfly/ci/pull/90#pullrequestreview-1605082967