045c47f172
Bazel operates by simply running the python scripts given to it in `py_test`. If the script doesn't invoke pytest on itself in the `if _name__ == "__main__"` snippet, no tests will be ran, and the script will pass. This has led to several tests (indeed, some are fixed in this PR) that, despite having been written, have never ran in CI. This PR adds a lint check to check all `py_test` sources for the presence of `if _name__ == "__main__"` snippet, and will fail CI if there are any detected without it. This system is only enabled for libraries right now (tune, train, air, rllib), but it could be trivially extended to other modules if approved. |
||
|---|---|---|
| .. | ||
| build | ||
| env | ||
| k8s | ||
| lint | ||
| pipeline | ||
| run | ||
| ci.sh | ||
| keep_alive | ||
| README.md | ||
| remote-watch.py | ||
| repro-ci.py | ||
| suppress_output | ||
CI process
This document is a work-in-progress. Please double-check file/function/etc. names for changes, as this document may be out of sync.
Dependencies
All dependencies (e.g. apt, pip) should be installed in install_dependencies(), following the same pattern as
those that already exist.
Once a dependency is added/removed, please ensure that shell environment variables are persisted appropriately, as CI
systems differ on when ~/.bashrc et al. are reloaded, if at all. (And they are not necessarily idempotent.)
Bazel, environment variables, and caching
Any environment variables passed to Bazel actions (e.g. PATH) should be idempotent to hit the Bazel cache.
If a different PATH gets passed to a Bazel action, Bazel will not hit the cache, and you might trigger a full rebuild
when you really expect an incremental (or no-op) build for an option (say pip install -e . after bazel build //...).
Invocation
The CI system (such as Travis) must source (not execute) ci/ci.sh and pass the action(s) to execute.
The script either handles the work or dispatches it to other script(s) as it deems appropriate.
This helps ensure any environment setup/teardown is handled appropriately.
Development best practices & pitfalls (read before adding a new script)
Before adding new scripts, please read this section.
First, please consider modifying an existing script instead (e.g. add your code as a separate function). Adding new scripts has a number of pitfalls that easily take hours (even days) to track down and fix:
-
When calling other scripts (as executables), environment variables (like
PATH) cannot propagate back up to the caller. Often, the caller expects such variables to be updated. -
When sourcing other scripts, global state (
ROOT_DIR,main,set -e, etc.) may be overwritten silently, causing unexpected behavior.
The following practices can avoid such pitfalls while maintaining intuitive control flow:
-
Put all environment-modifying functions in the top-level shell script, so that their invocation behaves intuitively. (The sheer length of the script is a secondary concern and can be mitigated by keeping functions modular.)
-
Avoid adding new scripts if possible. If it's necessary that you do so, call them instead of sourcing them. Note that this implies new scripts should not modify the environment, or the caller will not see such changes!
-
Always add code inside a function, not at global scope. Use
localfor variables where it makes sense. However, be careful and know the shell rules: for example, e.g.local x=$(false)succeeds even underset -e.
Ultimately, it's best to only add new scripts if they might need to be executed directly by non-CI code, as in that case, they should probably not use CI entrypoints (which assume exclusive control over the machine).