From 1a5eda53362a69f47e1e996682d744a9f3837761 Mon Sep 17 00:00:00 2001
From: Alex Anderson <191496+alxndrsn@users.noreply.github.com>
Date: Mon, 7 Oct 2024 09:34:45 +0300
Subject: [PATCH] nginx-tests: eliminate duplicate dockerfile (#741)

pros:

* nginx tests actually run against a real image

cons:

* slows fresh/CI builds by about 1 minute
---
 .github/workflows/test-nginx.yml   |  4 ++++
 test/nginx-test.dockerfile         | 28 ----------------------------
 test/nginx.test.docker-compose.yml |  2 +-
 test/run-tests.sh                  |  5 -----
 test/test-nginx.js                 | 13 +++++++------
 5 files changed, 12 insertions(+), 40 deletions(-)
 delete mode 100644 test/nginx-test.dockerfile

diff --git a/.github/workflows/test-nginx.yml b/.github/workflows/test-nginx.yml
index 72434a2c..ee56a757 100644
--- a/.github/workflows/test-nginx.yml
+++ b/.github/workflows/test-nginx.yml
@@ -10,6 +10,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v4
+      with:
+        fetch-depth: 0
+        fetch-tags: true
+        submodules: recursive
     - uses: actions/setup-node@v4
       with:
         node-version: 20.17.0
diff --git a/test/nginx-test.dockerfile b/test/nginx-test.dockerfile
deleted file mode 100644
index 6fe86da0..00000000
--- a/test/nginx-test.dockerfile
+++ /dev/null
@@ -1,28 +0,0 @@
-#
-# KEEP THIS FILE AS CLOSE AS POSSIBLE TO ../nginx.dockerfile
-#
-# It is ESPECIALLY IMPORTANT to keep IDENTICAL BASE IMAGES.
-#
-FROM jonasal/nginx-certbot:5.4.0
-
-EXPOSE 80
-EXPOSE 443
-
-# Persist Diffie-Hellman parameters and/or selfsign key
-VOLUME [ "/etc/dh", "/etc/selfsign" ]
-
-RUN apt-get update && apt-get install -y netcat-openbsd
-
-RUN mkdir -p /usr/share/odk/nginx/
-
-COPY files/nginx/setup-odk.sh /scripts/
-RUN chmod +x /scripts/setup-odk.sh
-
-COPY files/nginx/redirector.conf /usr/share/odk/nginx/
-COPY files/nginx/common-headers.conf /usr/share/odk/nginx/
-
-COPY ./test/files/nginx-test/http_root/ /usr/share/nginx/html
-COPY ./test/files/nginx-test/acme-challenge /var/www/letsencrypt/
-RUN ls /var/www/letsencrypt/
-
-ENTRYPOINT [ "/scripts/setup-odk.sh" ]
diff --git a/test/nginx.test.docker-compose.yml b/test/nginx.test.docker-compose.yml
index 1615c8f8..f7af4ace 100644
--- a/test/nginx.test.docker-compose.yml
+++ b/test/nginx.test.docker-compose.yml
@@ -16,7 +16,7 @@ services:
   nginx:
     build:
       context: ..
-      dockerfile: ./test/nginx-test.dockerfile
+      dockerfile: nginx.dockerfile
     depends_on:
       - service
       - enketo
diff --git a/test/run-tests.sh b/test/run-tests.sh
index 3833414f..f9ee7828 100755
--- a/test/run-tests.sh
+++ b/test/run-tests.sh
@@ -26,11 +26,6 @@ wait_for_http_response() {
   fi  
 }
 
-log "Checking nginx dockerfiles have same base image..."
-# It would be nice if Dockerfiles allowed some kind of templating which would
-# allow for direct sharing between the real and test nginx dockerfiles.
-diff <(grep FROM nginx-test.dockerfile) <(grep FROM ../nginx.dockerfile | grep -v AS)
-
 log "Starting test services..."
 docker_compose up --build --detach
 
diff --git a/test/test-nginx.js b/test/test-nginx.js
index 3731e027..e7288743 100644
--- a/test/test-nginx.js
+++ b/test/test-nginx.js
@@ -43,22 +43,24 @@ describe('nginx config', () => {
   });
 
   [
-    '/index.html',
-    '/version.txt',
-  ].forEach(staticFile => {
+    [ '/index.html',  /<div id="app"><\/div>/ ],
+    [ '/version.txt', /^versions:/ ],
+  ].forEach(([ staticFile, expectedContent ]) => {
     it(`${staticFile} file should have no-cache header`, async () => {
       // when
       const res = await fetchHttps(staticFile);
 
       // then
       assert.equal(res.status, 200);
-      assert.equal(await res.text(), `hi:${staticFile}\n`);
+      assert.match(await res.text(), expectedContent);
       assert.equal(await res.headers.get('cache-control'), 'no-cache');
     });
   });
 
   [
-    '/should-be-cached.txt',
+    '/blank.html',
+    '/favicon.ico',
+    // there's no way to predict generated asset paths, as they have cache-busting names
   ].forEach(staticFile => {
     it(`${staticFile} file should not have no-cache header`, async () => {
       // when
@@ -66,7 +68,6 @@ describe('nginx config', () => {
 
       // then
       assert.equal(res.status, 200);
-      assert.equal(await res.text(), `hi:${staticFile}\n`);
       assert.isNull(await res.headers.get('cache-control'));
     });
   });