Explain some steps of build.yml

tbergman · Oct 29, 2020 · f022ea5 · f022ea5
1 parent f30f191
commit f022ea5
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 0 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -102,6 +102,9 @@ jobs:
         with:
           key: db-${{ runner.os }}-${{ hashFiles('package/db/setup.py') }}
           path: package/db/cache/
+      # install prod versions of fiftyone-db and fiftyone-gui from source - this
+      # allows the build to succeed if requirements for those packages have been
+      # bumped but they have not been published yet
       - name: Install fiftyone-db
         working-directory: package/db
         run: |
@@ -124,6 +127,10 @@ jobs:
           PIP_INDEX_URL: https://voxel51-ci@pypi.voxel51.com
         run: |
           pip install -e . fiftyone-brain
+      # upgrade voxel51-eta to the latest pre-release (can't use the same
+      # strategy as fiftyone-db/gui above because it's in a different repo).
+      # Release branches and tags are excluded to force a proper compatible ETA
+      # release to be published first.
       - name: Upgrade ETA to pre-release
         if: ${{ !startsWith(github.ref, 'refs/heads/rel') && !startsWith(github.ref, 'refs/tags/') }}
         env:
@@ -135,6 +142,11 @@ jobs:
         if: ${{ !startsWith(matrix.os, 'windows') }}
         run: |
           pip install pytest tensorflow tensorflow-datasets torch torchvision
+      # obtained from "Windows, pip" instructions at
+      # https://pytorch.org/get-started/locally/ - unaware of a way to update
+      # the version number automatically, but tests on other platforms should
+      # hopefully catch any relevant PyTorch incompatibilities even if the
+      # PyTorch version used on Windows is not the latest
       - name: Install test dependencies (Windows)
         if: ${{ startsWith(matrix.os, 'windows') }}
         run: |
@@ -145,13 +157,20 @@ jobs:
         run: |
           python tests/utils/setup_config.py
           python tests/utils/github_actions_flags.py
+      # Important: use pytest_wrapper.py instead of pytest directly to ensure
+      # that services shut down cleanly and do not conflict with steps later in
+      # this workflow
       - name: Run tests
         run: |
           python tests/utils/pytest_wrapper.py tests/ --verbose --ignore tests/benchmarking/ --ignore tests/isolated/ --ignore tests/utils/ --ignore tests/import_export/
+      # Intended to run even if the tests above failed (todo: don't run if the
+      # tests were skipped due to an earlier step failing?)
       - name: Run isolated tests
         if: success() || failure()
         run: |
           find tests/isolated/ -name '*.py' -print0 | xargs -0 --verbose -n1 python tests/utils/pytest_wrapper.py --verbose
+      # these are somewhat slower/more bandwidth-intensive, so only run them if
+      # github_actions_flags.py determined that they should be run
       - name: Test quickstarts
         if: ${{ steps.test_config.outputs.run_integration == 'true' }}
         env:

diff --git a/package/README.md b/package/README.md
@@ -59,6 +59,14 @@ It is recommended to:
     _before_ publishing `fiftyone`. This ensures that users are never able to
     download a `fiftyone` package whose dependencies have not been published
     yet.
+    -   The test workflow currently installs the latest pre-release of
+        `voxel51-eta` if available (alpha/beta/rc) - this allows tests to use
+        bleeding-edge versions of ETA published to PyPI without making them
+        available to end-users by default. This is usually only necessary if
+        `fiftyone` tests fail without new ETA features. However, pre-releases
+        are intentionally not installed for tests run on release branches/tags,
+        so a stable release of `voxel51-eta` will need to be available before a
+        `fiftyone` release can be made.
 -   Create the tags on a release branch and wait for builds to be published
     successfully before merging the branch.
 -   Update `master` after the release has been merged into `develop`: