Clean up based on review comments

Author: mofojed

docker-compose.docs-snapshots.yml

@@ -1,20 +1,33 @@
-version: '3'
+volumes:
+  # This is a temporary directory that is used to store the test sets and results.
+  # It is mounted in the extractor and snapshotter containers.
+  # The results are then copied back to the plugins directories.
+  test-sets:
+    driver: local
+    driver_opts:
+      type: none
+      device: ./docker/build/test-sets
+      o: bind
 
 services:
-  deephaven-plugins-docs:
+  # Test server that has all the plugins loaded. Test sets are run against this server.
+  deephaven-plugins-docs-test-server:
     container_name: deephaven-plugins
     build:
       dockerfile: ./Dockerfile
       pull: true # We need to always pull the latest server image used in the Dockerfile
     ports:
       - '${DEEPHAVEN_SNAPSHOT_PORT:-10090}:10000'
     volumes:
+      # Override the /app.d/ directory with a blank directory. We don't want to run any of the default apps.
       - /app.d/
     environment:
       - START_OPTS=-Xmx4g -DAuthHandlers=io.deephaven.auth.AnonymousAuthenticationHandler -Ddeephaven.console.type=python -Ddeephaven.application.dir=./app.d
 
+  # Extract the code blocks from the docs in the plugins directories and output them to a results directory.
   deephaven-plugins-docs-extractor:
     image: ghcr.io/deephaven/salmon-extractor
+    pull_policy: always
     container_name: deephaven-plugins-docs-extractor
     command:
       [
@@ -25,25 +38,27 @@ services:
     volumes:
       - ./plugins/ui/docs/build/markdown:/extract/ui
       - ./plugins/plotly-express/docs/build/markdown:/extract/plotly-express
-      - ./docker/build/test-sets:/results # Output to a temporary build directory
+      - test-sets:/results # Output to a temporary build directory
 
+  # Read the test sets from the results directory and run them against the test server, taking a snapshot of the results.
   deephaven-plugins-docs-snapshotter:
     image: ghcr.io/deephaven/salmon-snapshotter
+    pull_policy: always
     depends_on:
-      deephaven-plugins-docs:
+      deephaven-plugins-docs-test-server:
         condition: service_healthy
       deephaven-plugins-docs-extractor:
         condition: service_completed_successfully
     environment:
-      HOST_URL: 'http://deephaven-plugins-docs:10000'
+      HOST_URL: 'http://deephaven-plugins-docs-test-server:10000'
     command:
       [
         'sh',
         '-c',
         'node snapshot.cjs --directory /test-sets/plotly-express --output /results/plotly-express; node snapshot.cjs --directory /test-sets/ui --output /results/ui',
       ]
     volumes:
-      - ./docker/build/test-sets:/test-sets
+      - test-sets:/test-sets
       # Map all the results back to the snapshots directory for those docs
       - ./plugins/ui/docs/snapshots:/results/ui
       - ./plugins/plotly-express/docs/snapshots:/results/plotly-express

plugins/plotly-express/docs/bar.md

@@ -16,8 +16,6 @@ Bar plots are appropriate when the data contain a continuous response variable t
 
 Visualize the relationship between a continuous variable and a categorical or discrete variable by passing the column names to the `x` and `y` arguments.
 
-<!-- TODO: Need to figure out how to get this info string through, without dropping it! Sphinx parser seems to just drop it -->
-
 ```python order=bar_plot,tips
 import deephaven.plot.express as dx
 tips = dx.data.tips()

plugins/ui/docs/components/markdown.md

@@ -90,7 +90,7 @@ def latex_markdown():
 latex_example = latex_markdown()
 ```
 
-<!-- TODO: Need to find a way to get this working correctly. Myst parser does not handle nested code blocks correctly.
+<!-- TODO (DOC-770): Need to find a way to get this working correctly. Myst parser does not handle nested code blocks correctly.
 ## Code Blocks
 
 Code blocks follow Deephaven's formatting.
@@ -146,7 +146,3 @@ style_example = style_markdown()
 ```{eval-rst}
 .. dhautofunction:: deephaven.ui.markdown
 ```
-
-```
-
-```

sphinx_ext/make_docs_utilities.py

@@ -136,11 +136,7 @@ def copy_snapshots() -> int:
         f"cp -r docs/snapshots {BUILT_DOCS}/snapshots",
     ]
 
-    code = attempt_command_sequence(commands)
-    if code != 0:
-        return 1
-
-    return 0
+    return attempt_command_sequence(commands)
 
 
 def build_documents() -> int:

tests/docker-compose.yml

@@ -17,14 +17,13 @@ services:
 
   e2e-tests:
     build:
-      context: ../
-      dockerfile: ./tests/Dockerfile
+      dockerfile: ./Dockerfile
       pull: true # We only use FROM with a pinned version here, but adding this just in case that changes in the future
     ports:
       - '9323:9323'
     ipc: host
     volumes:
-      - ../tests:/work/tests
+      - .:/work/tests
       - ../test-results:/work/test-results
       - ../playwright-report:/work/playwright-report
     entrypoint: 'npx playwright test --config=playwright-docker.config.ts'