Simplify e2e tests #33
Conversation
|
/gcbrun |
2 similar comments
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
becky-dt
changed the title
|
/gcbrun |
ghost
force-pushed
the
refactor/simplify-e2e-tests
branch
from
dd7829f to
9907264
Compare
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
There was a problem hiding this comment.
I'm happy with the changes, just noticed that test task isn't documented in this Makefile.
There was a problem hiding this comment.
I might have missed this during the rebase. @roberta-dt please could you take a look?
| make run pipeline=training build=false wait=true && \ | ||
| make run pipeline=prediction build=false wait=true |
There was a problem hiding this comment.
@browningjp-datatonic why don't we build containers when running e2e tests?
There was a problem hiding this comment.
The containers are built in the previous pipeline step. We pass build=false here because make build would create a separate Cloud Build job (which fails in this case, because it relies on the gcloud command that is not available in the CI container image)
| if args.wait.lower() == "true": | ||
| wait = True | ||
| elif args.wait.lower() != "false": | ||
| raise ValueError("wait variable must be 'true' or 'false'") | ||
| wait = False |
There was a problem hiding this comment.
@becky-dt can you please check for similar logic elsewhere in this repo? would like to follow the same logic or even import a reusable function for parsing boolean flags from the command-line. thanks!
There was a problem hiding this comment.
@felix-datatonic There are a couple more booleans in the Makefile but in this case the logic is all run in the makefile (unlike above where the wait param is passed into the python function)
Do we want to add similar logic in the makefile too to catch ValueErrors? and if so is there a good way to raise errors in bash
(let me know if this doesnt make sense and needs more explaining!)
There was a problem hiding this comment.
@roberta-dt and I added this logic to the makefile in the latest commit :)
There was a problem hiding this comment.
@browningjp-datatonic can you please take a look at the change we made to the run command?
There was a problem hiding this comment.
The other place where similar logic is used in Python is for ENABLE_PIPELINE_CACHING - however this is a bit different, because it's not just True/False, but True/False/None (None default)
The compile=true and build=true flags are implemented in the Makefile itself as you say, and it's not simple to replicate the logic exactly.
For consistency in behaviour, I would suggest to modify your Python logic here to:
if args.wait == "true":
wait = True
elif args.wait != "false":
raise ValueError("wait variable must be 'true' or 'false'")
wait = False(value from user must be true or false exactly - case sensitive)
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
Description
Removing e2e pytest tests and adding a wait in so that
make runwaits for the pipeline to finish before returning. Replacedmake e2e-testswithmake run wait=trueine2e-test.yamlCI pipelineHow has this been tested?
CI checks should be sufficient
Checklist