Merge pull request #911 from weather-gov/mgwalker/more-test-changes

End-to-end data testing via blocks
weather-gov · Mar 27, 2024 · 2678966 · 2678966
2 parents 277eeb8 + 198d66d
commit 2678966
Show file tree

Hide file tree

Showing 141 changed files with 89,786 additions and 6,299 deletions.
diff --git a/.github/workflows/code-standards.yaml b/.github/workflows/code-standards.yaml
@@ -158,109 +158,119 @@ jobs:
         if: needs.should-test.outputs.yes == 'true'
         run: npm run style-lint -- -f github
 
-  php-unit-tests:
-    name: PHP unit tests
+  build-drupal-image:
+    name: build Drupal image
     runs-on: ubuntu-latest
     needs: [should-test]
-    if: false
 
     steps:
       - name: checkout
         if: needs.should-test.outputs.yes == 'true'
         uses: actions/checkout@v4
 
-      - name: setup PHP
+      - name: setup image cacheing
         if: needs.should-test.outputs.yes == 'true'
-        uses: shivammathur/setup-php@v2
+        uses: actions/cache@v4
+        id: cache
         with:
-          php-version: "8.2"
-          coverage: xdebug
+          key: drupal-image-${{ hashFiles('composer.lock','web/sites/example.settings.dev.php','Dockerfile.dev') }}
+          path: /tmp/image.tar
 
-      - name: get composer paths
+      - name: Set up Docker Buildx
+        if: needs.should-test.outputs.yes == 'true' && steps.cache.outputs.cache-hit != 'true'
+        uses: docker/setup-buildx-action@v3
+
+      - name: build and export
+        if: needs.should-test.outputs.yes == 'true' && steps.cache.outputs.cache-hit != 'true'
+        uses: docker/build-push-action@v5
+        with:
+          context: .
+          file: ./Dockerfile.dev
+          tags: 18f-zscaler-drupal:10-apache
+          outputs: type=docker,dest=/tmp/image.tar
+
+  php-tests:
+    name: PHP tests
+    runs-on: ubuntu-latest
+    needs: [build-drupal-image, should-test]
+
+    steps:
+      - name: checkout
         if: needs.should-test.outputs.yes == 'true'
-        id: composer-paths
-        run: |
-          echo "cache-dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
-          echo "bin-dir=${{ github.workspace }}/$(composer config bin-dir)" >> $GITHUB_OUTPUT
+        uses: actions/checkout@v4
 
-      - name: cache composer caches
+      - name: setup image cacheing
         if: needs.should-test.outputs.yes == 'true'
         uses: actions/cache@v4
         with:
-          path: ${{ steps.composer-paths.outputs.cache-dir }}
-          key: composer-cache-${{ hashFiles('composer.lock') }}
+          key: drupal-image-${{ hashFiles('composer.lock','web/sites/example.settings.dev.php','Dockerfile.dev') }}
+          path: /tmp/image.tar
 
-      - name: install dependencies
+      - name: cache spatial data
         if: needs.should-test.outputs.yes == 'true'
-        env:
-          COMPOSER_NO_DEV: 0
-        run: composer install
+        uses: actions/cache@v4
+        with:
+          key: spatial-data-05mr24
+          path: |
+            spatial-data/c_05mr24.zip
+            spatial-data/s_05mr24.zip
+
+      - name: start the site
+        if: needs.should-test.outputs.yes == 'true'
+        run: |
+          mkdir .coverage
+          chmod a+rwx -R .coverage
+          docker load --input /tmp/image.tar
+          docker compose up -d
+
+      # Give the containers a moment to settle.
+      - name: wait a tick
+        if: needs.should-test.outputs.yes == 'true'
+        run: sleep 10
+
+      - name: populate the site
+        if: needs.should-test.outputs.yes == 'true'
+        run: |
+          cp web/sites/example.settings.dev.php web/sites/settings.dev.php
+          make install-site
+          make load-spatial
 
       - name: run unit tests
         if: needs.should-test.outputs.yes == 'true'
         run: |
-          vendor/bin/phpunit --coverage-clover coverage.xml
+          curl http://localhost:8081/play/e2e
+          make backend-test
 
       - name: store coverage output
         if: needs.should-test.outputs.yes == 'true'
         uses: actions/upload-artifact@v4
         with:
           name: coverage-report
-          path: coverage.xml
+          path: .coverage/clover.xml
           retention-days: 1
 
   min-code-coverage:
     name: "90% code coverage"
     runs-on: ubuntu-latest
-    needs: [php-unit-tests, should-test]
+    needs: [php-tests, should-test]
 
     steps:
       - name: get coverage output
-        if: false
+        id: download
+        if: needs.should-test.outputs.yes == 'true'
         uses: actions/download-artifact@v4
         with:
           name: coverage-report
 
       - name: 90% code coverage
-        if: false
+        if: needs.should-test.outputs.yes == 'true'
         id: test-coverage
         uses: johanvanhelden/gha-clover-test-coverage-check@v1
         with:
           percentage: 90
-          filename: coverage.xml
+          filename: clover.xml
           metric: statements
 
-  build-drupal-image:
-    name: build Drupal image
-    runs-on: ubuntu-latest
-    needs: [should-test]
-
-    steps:
-      - name: checkout
-        if: needs.should-test.outputs.yes == 'true'
-        uses: actions/checkout@v4
-
-      - name: setup image cacheing
-        if: needs.should-test.outputs.yes == 'true'
-        uses: actions/cache@v4
-        id: cache
-        with:
-          key: drupal-image-${{ hashFiles('composer.lock','web/sites/example.settings.dev.php','Dockerfile.dev') }}
-          path: /tmp/image.tar
-
-      - name: Set up Docker Buildx
-        if: needs.should-test.outputs.yes == 'true' && steps.cache.outputs.cache-hit != 'true'
-        uses: docker/setup-buildx-action@v3
-
-      - name: build and export
-        if: needs.should-test.outputs.yes == 'true' && steps.cache.outputs.cache-hit != 'true'
-        uses: docker/build-push-action@v5
-        with:
-          context: .
-          file: ./Dockerfile.dev
-          tags: 18f-zscaler-drupal:10-apache
-          outputs: type=docker,dest=/tmp/image.tar
-
   end-to-end-tests:
     name: end-to-end tests
     runs-on: ubuntu-latest

diff --git a/Makefile b/Makefile
@@ -93,17 +93,22 @@ a11y: accessibility-test
 accessibility-test: ## Run accessibility tests (alias a11y)
 	npx cypress run --project tests/a11y
 
+be: backend-test
+backend-test: ## Run all backend tests. (alias be)
+	docker compose exec drupal phpunit --group unit,e2e --process-isolation --coverage-html /coverage  --coverage-clover /coverage/clover.xml
+
 ee: end-to-end-test
 end-to-end-test: ## Run end-to-end tests in Cypress. (alias ee)
 	npx cypress run --project tests/e2e
+	docker compose exec drupal phpunit --group e2e --process-isolation
 
 lt: load-time-test
 load-time-test: ## Run page load time tests in Cypress (alias lt)
 	npx cypress run --project tests/load-times
 
 u: unit-test
 unit-test: ## Run PHP unit tests
-	docker compose exec drupal phpunit --coverage-html /coverage
+	docker compose exec drupal phpunit --group unit
 
 ### Linting
 js-lint: ## Run eslint on our Javascript

diff --git a/docs/dev/testing.md b/docs/dev/testing.md
@@ -10,23 +10,81 @@ we've chosen.
 > documented in
 > [architectural decision record #9](../architecture/decisions/0009-we-will-use-a-content-management-system.md).
 
-## PHP unit testing
+## PHP testing
+
+### Unit tests
+
+We use [PHPUnit](https://phpunit.de/) for unit testing utility classes and
+custom block types. These particular classes are relatively isolated so don't
+require extensive mocking.
+
+For blocks in particular, there is a base test class that handles creating the
+block under test and setting up the appropriate mocks that are
+dependency-injected. It also provides a couple of helpful behaviors:
+
+1. By default, it makes the `getLocation` method mockable. Calling
+   `$this->onLocationRoute()` in a block test will configure the `getLocation`
+   mock to return an appropriate location object with grid and point properties
+   already set. Similarly, `$this->notOnLocationRoute()` will mock `getLocation`
+   to return a location with the grid and point set to `false`.
+
+2. Automatically adds a test that mocks all of the WeatherDataService methods to
+   throw exceptions and tests that the block returns `["error" => true]` in
+   those cases.
+   > [!NOTE]  
+   > If your block doesn't need this test, you can override it:
+   >
+   > ```php
+   > public function testHandlesExceptions(): void
+   > {
+   >   $this->assertEquals(true, true);
+   > }
+   > ```
+
+To run just these unit tests, run:
+
+```shell
+make unit-test
+...or...
+make u
+```
+
+### "API" end-to-end testing
+
+Our Drupal services are largely concerned with fetching data from external
+sources (such as the API or the database) and formatting it to our needs. Unit
+testing them would require a lot of complex and fragile mocking. (We did this
+initially and it was a nightmare to maintain). Instead, we have opted to test
+our services against live data using our API proxy tool.
+
+We essentially treat our blocks as API endpoints. We load a block, point it at
+a particular location with known data, and then ensure the data comes back as
+expected. This is much like end-to-end testing.
 
-For our Drupal modules, we test the PHP code with unit tests. These allow us to
-test the code in isolation and independently. We use
-[PHPUnit](https://phpunit.de/) for these tests, and we rely on PHPUnit's
-built-in "[test doubles](https://docs.phpunit.de/en/9.6/test-doubles.html)"
-feature to mock dependencies and Drupal's
-[dependency
-injection system](https://www.drupal.org/docs/drupal-apis/services-and-dependency-injection/services-and-dependency-injection-in-drupal-8)
-to get our mocks into the code under test.
+These tests are also implemented using [PHPUnit](https://phpunit.de/). They are
+stored alongside our blocks code, since they are executing blocks. They should
+extend the `EndToEndBase` class, which handles setting up autoloading and
+creating all of the necessary services and mocks to run the tests.
 
-To run unit tests locally, the Makefile command is:
+It also provides an `onLocationRoute` helper method like the block unit test
+base class. However, instead of mocking the base `getLocation` method, it
+mocks the RouteMatchInterface object to identify the route as being at a given
+location. This ensures more thorough testing.
+
+To defeat caching, these tests are run in process isolation mode, meaning each
+test runs in its own process. This results in somewhat slower tests, but it
+also means we don't have to deal with trying to bypass caches in testing.
+
+To run these tests locally, the Makefile command is:
 
 ```sh
-make u
+make backend-test
+...or...
+make be
 ```
 
+Note that backend tests include unit tests.
+
 ## End-to-end testing
 
 We test the running product using [Cypress](https://www.cypress.io/) to load