From 9962256699d6f98967379a6d33d938ba165df5d2 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Tue, 21 May 2024 10:27:15 +0100 Subject: [PATCH 01/17] Adding screenshot to test --- .github/workflows/deploy.yml | 4 ++++ package.json | 2 +- test/PageObjectModels/uploadFilePage.js | 8 ++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 91271f60..66d0eb9f 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -67,6 +67,10 @@ jobs: - id: run-acceptance-tests run: npm run test:acceptance:ci + + - name: Upload screenshot artifact + uses: actions/upload-artifact@v2 + - id: stop-containers if: always() diff --git a/package.json b/package.json index e95d4f86..f155135d 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "test:coverage": "vitest test/unit test/integration test/contract --coverage", "test:integration": "NODE_ENV=test playwright test --config ./test/integration/playwright.config.js", "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js", - "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @url", + "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @datafille", "playwright-codegen": "playwright codegen http://localhost:5000", "lint": "standard", "lint:fix": "standard --fix" diff --git a/test/PageObjectModels/uploadFilePage.js b/test/PageObjectModels/uploadFilePage.js index 56935eaf..6bb3d13a 100644 --- a/test/PageObjectModels/uploadFilePage.js +++ b/test/PageObjectModels/uploadFilePage.js @@ -1,5 +1,6 @@ import BasePage from './BasePage' import StatusPage from './statusPage' +import fs from 'fs' export default class UploadFilePage extends BasePage { constructor (page) { @@ -11,10 +12,17 @@ export default class UploadFilePage extends BasePage { await this.page.getByText('Upload data').click() const fileChooser = await fileChooserPromise await fileChooser.setFiles(filePath) + } async clickContinue (skipVerification) { await super.clickContinue() + await this.page.waitForTimeout(10000) + const screenshotPath = 'test/datafiles/screenshot.png'; + const screenshotBuffer = await this.page.screenshot(); + fs.writeFileSync(screenshotPath, screenshotBuffer); + console.log(`Screenshot saved to: ${screenshotPath}`); + console.log("URL here:",this.page.url()) return await super.verifyAndReturnPage(StatusPage, skipVerification) } } From 5ba3cb7de90568e4adb502a06c655beadf5bf80e Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Tue, 21 May 2024 10:27:35 +0100 Subject: [PATCH 02/17] lint fix --- test/PageObjectModels/uploadFilePage.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/PageObjectModels/uploadFilePage.js b/test/PageObjectModels/uploadFilePage.js index 6bb3d13a..cd7e40d8 100644 --- a/test/PageObjectModels/uploadFilePage.js +++ b/test/PageObjectModels/uploadFilePage.js @@ -12,17 +12,16 @@ export default class UploadFilePage extends BasePage { await this.page.getByText('Upload data').click() const fileChooser = await fileChooserPromise await fileChooser.setFiles(filePath) - } async clickContinue (skipVerification) { await super.clickContinue() await this.page.waitForTimeout(10000) - const screenshotPath = 'test/datafiles/screenshot.png'; - const screenshotBuffer = await this.page.screenshot(); - fs.writeFileSync(screenshotPath, screenshotBuffer); - console.log(`Screenshot saved to: ${screenshotPath}`); - console.log("URL here:",this.page.url()) + const screenshotPath = 'test/datafiles/screenshot.png' + const screenshotBuffer = await this.page.screenshot() + fs.writeFileSync(screenshotPath, screenshotBuffer) + console.log(`Screenshot saved to: ${screenshotPath}`) + console.log('URL here:', this.page.url()) return await super.verifyAndReturnPage(StatusPage, skipVerification) } } From 7698268d7e213ef94ff947bf1a9fe90ef11bc059 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Tue, 21 May 2024 10:38:29 +0100 Subject: [PATCH 03/17] typo mistake --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f155135d..69f466b6 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "test:coverage": "vitest test/unit test/integration test/contract --coverage", "test:integration": "NODE_ENV=test playwright test --config ./test/integration/playwright.config.js", "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js", - "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @datafille", + "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @datafile", "playwright-codegen": "playwright codegen http://localhost:5000", "lint": "standard", "lint:fix": "standard --fix" From 304eea844f9d9123afb59f61d18616da1d4f4e15 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Tue, 21 May 2024 11:31:40 +0100 Subject: [PATCH 04/17] Adding test file at root --- ...cle4directionareas-ok.csv => article4directionareas-ok.csv | 0 package.json | 4 ++-- test/acceptance/request_check.test.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename test/datafiles/article4directionareas-ok.csv => article4directionareas-ok.csv (100%) diff --git a/test/datafiles/article4directionareas-ok.csv b/article4directionareas-ok.csv similarity index 100% rename from test/datafiles/article4directionareas-ok.csv rename to article4directionareas-ok.csv diff --git a/package.json b/package.json index 69f466b6..87e015cc 100644 --- a/package.json +++ b/package.json @@ -24,8 +24,8 @@ "test:watch": "vitest test/unit test/integration test/contract", "test:coverage": "vitest test/unit test/integration test/contract --coverage", "test:integration": "NODE_ENV=test playwright test --config ./test/integration/playwright.config.js", - "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js", - "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @datafile", + "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js --grep @datafile1", + "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @datafile1", "playwright-codegen": "playwright codegen http://localhost:5000", "lint": "standard", "lint:fix": "standard --fix" diff --git a/test/acceptance/request_check.test.js b/test/acceptance/request_check.test.js index 3060ad60..10902987 100644 --- a/test/acceptance/request_check.test.js +++ b/test/acceptance/request_check.test.js @@ -16,7 +16,7 @@ import { uploadMethods } from '../PageObjectModels/uploadMethodPage' test.setTimeout(50000) test.describe('Request Check', () => { test.describe('with javascript enabled', () => { - test('request check of a @datafile', async ({ page }) => { + test('request check of a @datafile1', async ({ page }) => { const startPage = new StartPage(page) await startPage.navigateHere() @@ -31,7 +31,7 @@ test.describe('Request Check', () => { const uploadFilePage = await uploadMethodPage.clickContinue() await uploadFilePage.waitForPage() - await uploadFilePage.uploadFile('test/datafiles/article4directionareas-ok.csv') + await uploadFilePage.uploadFile('./article4directionareas-ok.csv') const statusPage = await uploadFilePage.clickContinue() await statusPage.waitForPage() From bfa849a552bb5a01418da889da028c4b8b31bdee Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Tue, 21 May 2024 11:37:30 +0100 Subject: [PATCH 05/17] Adding upload artifact --- .github/workflows/deploy.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 66d0eb9f..dfdc79a4 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -68,8 +68,12 @@ jobs: - id: run-acceptance-tests run: npm run test:acceptance:ci - - name: Upload screenshot artifact - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v3 + if: always() + with: + name: playwright-report + path: playwright-report/ + retention-days: 30 - id: stop-containers From 75d34c611e70cbc7ecfef34b9687e4600028ea0e Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Tue, 21 May 2024 12:16:32 +0100 Subject: [PATCH 06/17] Adding path for SS --- test/PageObjectModels/uploadFilePage.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/PageObjectModels/uploadFilePage.js b/test/PageObjectModels/uploadFilePage.js index cd7e40d8..fe97cac7 100644 --- a/test/PageObjectModels/uploadFilePage.js +++ b/test/PageObjectModels/uploadFilePage.js @@ -1,6 +1,5 @@ import BasePage from './BasePage' import StatusPage from './statusPage' -import fs from 'fs' export default class UploadFilePage extends BasePage { constructor (page) { @@ -16,11 +15,8 @@ export default class UploadFilePage extends BasePage { async clickContinue (skipVerification) { await super.clickContinue() - await this.page.waitForTimeout(10000) - const screenshotPath = 'test/datafiles/screenshot.png' - const screenshotBuffer = await this.page.screenshot() - fs.writeFileSync(screenshotPath, screenshotBuffer) - console.log(`Screenshot saved to: ${screenshotPath}`) + await this.page.waitForTimeout(5000) + await this.page.screenshot({ path: 'playwright-report/data/screenshot.png', fullPage: true }) console.log('URL here:', this.page.url()) return await super.verifyAndReturnPage(StatusPage, skipVerification) } From 0fff1d5d12e1d323ebc96b8facfd4d5f79c92d25 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Wed, 22 May 2024 10:10:39 +0100 Subject: [PATCH 07/17] Adding logs for frontend and request-api --- .github/workflows/deploy.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index dfdc79a4..7ecbc551 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -75,6 +75,12 @@ jobs: path: playwright-report/ retention-days: 30 + - id: docker-compose-logs + if: always() + run: | + sleep 30 + docker compose -f docker-compose-real-backend.yml logs --tail frontend + docker compose -f docker-compose-real-backend.yml logs --tail request-api - id: stop-containers if: always() From 34f5dc0e46a6cf7a36f9424d358aea7fd5629271 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Wed, 22 May 2024 16:55:07 +0100 Subject: [PATCH 08/17] Pushing url test cases only --- .github/workflows/deploy.yml | 6 ------ package.json | 4 ++-- test/acceptance/request_check.test.js | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 7ecbc551..dfdc79a4 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -75,12 +75,6 @@ jobs: path: playwright-report/ retention-days: 30 - - id: docker-compose-logs - if: always() - run: | - sleep 30 - docker compose -f docker-compose-real-backend.yml logs --tail frontend - docker compose -f docker-compose-real-backend.yml logs --tail request-api - id: stop-containers if: always() diff --git a/package.json b/package.json index 87e015cc..6932517d 100644 --- a/package.json +++ b/package.json @@ -24,8 +24,8 @@ "test:watch": "vitest test/unit test/integration test/contract", "test:coverage": "vitest test/unit test/integration test/contract --coverage", "test:integration": "NODE_ENV=test playwright test --config ./test/integration/playwright.config.js", - "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js --grep @datafile1", - "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @datafile1", + "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js --grep @url", + "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @url", "playwright-codegen": "playwright codegen http://localhost:5000", "lint": "standard", "lint:fix": "standard --fix" diff --git a/test/acceptance/request_check.test.js b/test/acceptance/request_check.test.js index 10902987..43d3f137 100644 --- a/test/acceptance/request_check.test.js +++ b/test/acceptance/request_check.test.js @@ -16,7 +16,7 @@ import { uploadMethods } from '../PageObjectModels/uploadMethodPage' test.setTimeout(50000) test.describe('Request Check', () => { test.describe('with javascript enabled', () => { - test('request check of a @datafile1', async ({ page }) => { + test('request check of a @datafile', async ({ page }) => { const startPage = new StartPage(page) await startPage.navigateHere() From 2e40de0564211570027c0f5a2f21844e237c9df9 Mon Sep 17 00:00:00 2001 From: Chris Cundill Date: Wed, 22 May 2024 17:42:41 +0100 Subject: [PATCH 09/17] Added environment variables which are needed for AWS SDK to talk to localstack (needs placeholder AWS credentials set in environment) --- docker-compose-real-backend.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docker-compose-real-backend.yml b/docker-compose-real-backend.yml index 381700f1..7777ab30 100644 --- a/docker-compose-real-backend.yml +++ b/docker-compose-real-backend.yml @@ -6,6 +6,10 @@ services: context: . environment: NODE_ENV: ci + AWS_ACCESS_KEY_ID: example + AWS_SECRET_ACCESS_KEY: example + AWS_DEFAULT_REGION: eu-west-2 + # AWS_ENDPOINT_URL: http://localstack:4566 ports: - "8085:5000" From 831317dcf2ad27efcf569fd392da15750868ea83 Mon Sep 17 00:00:00 2001 From: Chris Cundill Date: Thu, 23 May 2024 08:16:21 +0100 Subject: [PATCH 10/17] Changed filter on acceptance tests to include both url and file scenarios --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 6932517d..8955bbc4 100644 --- a/package.json +++ b/package.json @@ -24,8 +24,8 @@ "test:watch": "vitest test/unit test/integration test/contract", "test:coverage": "vitest test/unit test/integration test/contract --coverage", "test:integration": "NODE_ENV=test playwright test --config ./test/integration/playwright.config.js", - "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js --grep @url", - "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js --grep @url", + "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js", + "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js", "playwright-codegen": "playwright codegen http://localhost:5000", "lint": "standard", "lint:fix": "standard --fix" From 3335358539ca3e897850f914620883a36d0aab50 Mon Sep 17 00:00:00 2001 From: Chris Cundill Date: Thu, 23 May 2024 10:04:29 +0100 Subject: [PATCH 11/17] Added additonal environment variable for frontend which might be required for communicating with localstack (AWS stub) --- docker-compose-real-backend.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose-real-backend.yml b/docker-compose-real-backend.yml index 7777ab30..0510d72b 100644 --- a/docker-compose-real-backend.yml +++ b/docker-compose-real-backend.yml @@ -9,7 +9,7 @@ services: AWS_ACCESS_KEY_ID: example AWS_SECRET_ACCESS_KEY: example AWS_DEFAULT_REGION: eu-west-2 - # AWS_ENDPOINT_URL: http://localstack:4566 + AWS_ENDPOINT_URL: http://localstack:4566 ports: - "8085:5000" From 66614c9803a21ad866a85e5f0d06db37bf55aa12 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Thu, 23 May 2024 10:36:56 +0100 Subject: [PATCH 12/17] Adding logs back --- .github/workflows/deploy.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index dfdc79a4..0d3e2285 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -75,6 +75,12 @@ jobs: path: playwright-report/ retention-days: 30 + - id: docker-compose-logs + if: always() + run: | + docker compose -f docker-compose-real-backend.yml logs --tail frontend + docker compose -f docker-compose-real-backend.yml logs --tail request-api + - id: stop-containers if: always() From db5622957fd1566cd9881a01cce94c27630e92b5 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Fri, 24 May 2024 09:17:59 +0100 Subject: [PATCH 13/17] Adding the file back to datafiles folder --- .../datafiles/article4directionareas-ok.csv | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename article4directionareas-ok.csv => test/datafiles/article4directionareas-ok.csv (100%) diff --git a/article4directionareas-ok.csv b/test/datafiles/article4directionareas-ok.csv similarity index 100% rename from article4directionareas-ok.csv rename to test/datafiles/article4directionareas-ok.csv From b8d0e8633eaa4cd028b97340563118a0ac442d60 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Fri, 24 May 2024 09:36:05 +0100 Subject: [PATCH 14/17] path back to test/datafiles --- test/acceptance/request_check.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/request_check.test.js b/test/acceptance/request_check.test.js index 43d3f137..3060ad60 100644 --- a/test/acceptance/request_check.test.js +++ b/test/acceptance/request_check.test.js @@ -31,7 +31,7 @@ test.describe('Request Check', () => { const uploadFilePage = await uploadMethodPage.clickContinue() await uploadFilePage.waitForPage() - await uploadFilePage.uploadFile('./article4directionareas-ok.csv') + await uploadFilePage.uploadFile('test/datafiles/article4directionareas-ok.csv') const statusPage = await uploadFilePage.clickContinue() await statusPage.waitForPage() From fddfe9305bc137f9b2006d494829e5e642c33848 Mon Sep 17 00:00:00 2001 From: Chris Cundill Date: Tue, 28 May 2024 07:47:49 +0100 Subject: [PATCH 15/17] Removed object-scope state from controllers since they are actually singleton instances. Using req.form.options to store request-scope state instead. This along with a few other tweaks should have the acceptance tests passing in the CI pipeline. Unit and integration have been updated accordingly for the state management changes in the controllers. --- docker-compose-real-backend.yml | 4 +- localstack_bootstrap/s3_bootstrap.sh | 26 +++++ src/controllers/resultsController.js | 61 +++++------ src/controllers/statusController.js | 15 +-- src/controllers/submitUrlController.js | 2 - src/controllers/uploadController.js | 15 --- src/controllers/uploadFileController.js | 2 - test/acceptance/request_check.test.js | 11 +- test/integration/playwright.config.js | 2 +- test/unit/resultsController.test.js | 138 +++++++++++------------- test/unit/statusController.test.js | 27 +---- test/unit/uploadController.test.js | 12 --- 12 files changed, 140 insertions(+), 175 deletions(-) create mode 100755 localstack_bootstrap/s3_bootstrap.sh diff --git a/docker-compose-real-backend.yml b/docker-compose-real-backend.yml index 0510d72b..a2e9ca78 100644 --- a/docker-compose-real-backend.yml +++ b/docker-compose-real-backend.yml @@ -37,6 +37,7 @@ services: request-api: image: public.ecr.aws/l6z6v3j6/development-pub-async-request-api:main + # image: request-api:latest # Uncomment when working locally with this file environment: PYTHONUNBUFFERED: 1 AWS_ENDPOINT_URL: http://localstack:4566 @@ -59,6 +60,7 @@ services: request-processor: image: public.ecr.aws/l6z6v3j6/development-pub-async-request-processor:main + # image: request-processor:latest # Uncomment when working locally with this file environment: PYTHONUNBUFFERED: 1 AWS_ENDPOINT_URL: http://localstack:4566 @@ -73,7 +75,7 @@ services: REQUEST_FILES_BUCKET_NAME: dluhc-data-platform-request-files-local restart: on-failure deploy: - replicas: 1 + replicas: 2 volumes: - "./request-processor-celery/docker_volume:/opt" diff --git a/localstack_bootstrap/s3_bootstrap.sh b/localstack_bootstrap/s3_bootstrap.sh new file mode 100755 index 00000000..913faa80 --- /dev/null +++ b/localstack_bootstrap/s3_bootstrap.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# enable debug +# set -x + +echo "configuring s3" +echo "===================" +LOCALSTACK_HOST=localhost +AWS_REGION=eu-west-2 + +create_upload_bucket() { + local BUCKET_NAME_TO_CREATE=$1 + awslocal --endpoint-url=http://${LOCALSTACK_HOST}:4566 s3api create-bucket --bucket ${BUCKET_NAME_TO_CREATE} --region ${AWS_REGION} --create-bucket-configuration LocationConstraint=${AWS_REGION} + awslocal --endpoint-url=http://${LOCALSTACK_HOST}:4566 s3api put-bucket-cors --bucket ${BUCKET_NAME_TO_CREATE} --cors-configuration file:///etc/localstack/init/ready.d/cors-config.json +} + +upload_file_to_bucket() { + local FILENAME=$1 + local FILEPATH=$2 + local BUCKET_NAME=$3 + awslocal s3api put-object --bucket ${BUCKET_NAME} --key ${FILENAME} --body ${FILEPATH} +} + +create_upload_bucket "dluhc-data-platform-request-files-local" diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index 13188a51..0be4575d 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -6,50 +6,47 @@ const errorsTemplate = 'results/errors' const noErrorsTemplate = 'results/no-errors' class ResultsController extends PageController { - async configure (req, res, next) { + async locals (req, res, next) { try { - this.result = await getRequestData(req.params.id) - if (!this.result.isComplete()) { + const result = await getRequestData(req.params.id) + req.form.options.data = result + + if (!result.isComplete()) { res.redirect(`/status/${req.params.id}`) return - } else if (this.result.isFailed()) { - this.template = failedRequestTemplate - } else if (this.result.hasErrors()) { - this.template = errorsTemplate - await this.result.fetchResponseDetails(req.params.pageNumber, 50, 'error') + } else if (result.isFailed()) { + req.form.options.template = failedRequestTemplate + } else if (result.hasErrors()) { + req.form.options.template = errorsTemplate + await result.fetchResponseDetails(req.params.pageNumber, 50, 'error') + } else { + req.form.options.template = noErrorsTemplate + await result.fetchResponseDetails(req.params.pageNumber) + } + + req.form.options.requestParams = result.getParams() + + if (req.form.options.template !== failedRequestTemplate) { + req.form.options.errorSummary = result.getErrorSummary() + req.form.options.columns = result.getColumns() + req.form.options.fields = result.getFields() + req.form.options.mappings = result.getFieldMappings() + req.form.options.verboseRows = result.getRowsWithVerboseColumns(result.hasErrors()) + req.form.options.geometries = result.getGeometries() + req.form.options.pagination = result.getPagination(req.params.pageNumber) + req.form.options.id = req.params.id } else { - this.template = noErrorsTemplate - await this.result.fetchResponseDetails(req.params.pageNumber) + req.form.options.error = result.getError() } - super.configure(req, res, next) + super.locals(req, res, next) } catch (error) { next(error, req, res, next) } } - async locals (req, res, next) { - req.form.options.template = this.template - req.form.options.requestParams = this.result.getParams() - - if (this.template !== failedRequestTemplate) { - req.form.options.errorSummary = this.result.getErrorSummary() - req.form.options.columns = this.result.getColumns() - req.form.options.fields = this.result.getFields() - req.form.options.mappings = this.result.getFieldMappings() - req.form.options.verboseRows = this.result.getRowsWithVerboseColumns(this.result.hasErrors()) - req.form.options.geometries = this.result.getGeometries() - req.form.options.pagination = this.result.getPagination(req.params.pageNumber) - req.form.options.id = req.params.id - } else { - req.form.options.error = this.result.getError() - } - - super.locals(req, res, next) - } - noErrors (req, res, next) { - return !this.result.hasErrors() + return !req.form.options.data.hasErrors() } } diff --git a/src/controllers/statusController.js b/src/controllers/statusController.js index 1b4132a7..e357b3ab 100644 --- a/src/controllers/statusController.js +++ b/src/controllers/statusController.js @@ -3,21 +3,16 @@ import { getRequestData } from '../utils/asyncRequestApi.js' import { finishedProcessingStatuses } from '../utils/utils.js' class StatusController extends PageController { - async configure (req, res, next) { + async locals (req, res, next) { try { - this.result = await getRequestData(req.params.id) - super.configure(req, res, next) + req.form.options.data = await getRequestData(req.params.id) + req.form.options.processingComplete = finishedProcessingStatuses.includes(req.form.options.data.status) + req.form.options.pollingEndpoint = `/api/status/${req.form.options.data.id}` + super.locals(req, res, next) } catch (error) { next(error, req, res, next) } } - - async locals (req, res, next) { - req.form.options.data = this.result - req.form.options.processingComplete = finishedProcessingStatuses.includes(this.result.status) - req.form.options.pollingEndpoint = `/api/status/${this.result.id}` - super.locals(req, res, next) - } } export default StatusController diff --git a/src/controllers/submitUrlController.js b/src/controllers/submitUrlController.js index b12bcd58..7b44180e 100644 --- a/src/controllers/submitUrlController.js +++ b/src/controllers/submitUrlController.js @@ -7,8 +7,6 @@ import { allowedFileTypes } from '../utils/utils.js' class SubmitUrlController extends UploadController { async post (req, res, next) { - this.resetValidationErrorMessage() - const localValidationErrorType = await SubmitUrlController.localUrlValidation(req.body.url) if (localValidationErrorType) { diff --git a/src/controllers/uploadController.js b/src/controllers/uploadController.js index 7f2926b7..ffebd4d0 100644 --- a/src/controllers/uploadController.js +++ b/src/controllers/uploadController.js @@ -1,29 +1,14 @@ 'use strict' import PageController from './pageController.js' import config from '../../config/index.js' -import logger from '../utils/logger.js' class UploadController extends PageController { apiRoute = config.asyncRequestApi.url + config.asyncRequestApi.requestsEndpoint - locals (req, res, next) { - req.form.options.validationError = this.validationErrorMessage - super.locals(req, res, next) - } - async post (req, res, next) { super.post(req, res, next) } - resetValidationErrorMessage () { - this.validationErrorMessage = undefined - } - - validationError (type, message, errorObject, req) { - logger.error({ type, message, errorObject }) - this.validationErrorMessage = message - } - getBaseFormData (req) { return { dataset: req.sessionModel.get('dataset'), diff --git a/src/controllers/uploadFileController.js b/src/controllers/uploadFileController.js index 5ccd8f46..8b464e46 100644 --- a/src/controllers/uploadFileController.js +++ b/src/controllers/uploadFileController.js @@ -26,8 +26,6 @@ class UploadFileController extends UploadController { } async post (req, res, next) { - this.resetValidationErrorMessage() - let dataFileForLocalValidation = null if (req.file) { diff --git a/test/acceptance/request_check.test.js b/test/acceptance/request_check.test.js index 3060ad60..e2b12152 100644 --- a/test/acceptance/request_check.test.js +++ b/test/acceptance/request_check.test.js @@ -13,7 +13,12 @@ import StartPage from '../PageObjectModels/startPage' import { datasets } from '../PageObjectModels/datasetPage' import { uploadMethods } from '../PageObjectModels/uploadMethodPage' -test.setTimeout(50000) +test.setTimeout(300000) + +// test.beforeEach(async ({ page }, testInfo) => { +// test.setTimeout(Number("60,000")); +// }); + test.describe('Request Check', () => { test.describe('with javascript enabled', () => { test('request check of a @datafile', async ({ page }) => { @@ -229,7 +234,7 @@ test.describe('Request Check', () => { await statusPage.expectCheckStatusButtonToBeVisible() const id = await statusPage.getIdFromUrl() - await page.waitForTimeout(3000) // wait for 3 seconds for processing. could be smarter about this so we dont have to wait 3 seconds + await page.waitForTimeout(5000) // wait for 10 seconds for processing. could be smarter about this so we dont have to wait 3 seconds const resultsPage = await statusPage.clickCheckStatusButton() @@ -260,7 +265,7 @@ test.describe('Request Check', () => { await statusPage.expectCheckStatusButtonToBeVisible() const id = await statusPage.getIdFromUrl() - await page.waitForTimeout(3000) // wait for 3 seconds for processing. could be smarter about this so we dont have to wait 3 seconds + await page.waitForTimeout(5000) // wait for 5 seconds for processing. could be smarter about this so we dont have to wait 3 seconds const resultsPage = await statusPage.clickCheckStatusButton() diff --git a/test/integration/playwright.config.js b/test/integration/playwright.config.js index 082f4f22..e7a2c612 100644 --- a/test/integration/playwright.config.js +++ b/test/integration/playwright.config.js @@ -86,7 +86,7 @@ export default defineConfig({ /* Run your local dev server before starting the tests */ webServer: { command: 'NODE_ENV=test npm run start', - url: 'http://127.0.0.1:5000', + url: `http://127.0.0.1:${config.port}`, reuseExistingServer: !process.env.CI } }) diff --git a/test/unit/resultsController.test.js b/test/unit/resultsController.test.js index 9b7fff72..05e9a848 100644 --- a/test/unit/resultsController.test.js +++ b/test/unit/resultsController.test.js @@ -20,74 +20,12 @@ describe('ResultsController', () => { }) }) - describe('configure', () => { - it('should add the result to the controller class', async () => { - const mockResult = { hasErrors: () => false } - asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) - - await resultsController.configure(req, {}, () => {}) - expect(resultsController.result).toBeDefined() - }) - - it("should call next with a 404 error if the result wasn't found", async () => { - asyncRequestApi.getRequestData = vi.fn().mockImplementation(() => { - throw new Error('Request not found', { message: 'Request not found', status: 404 }) - }) - - const nextMock = vi.fn() - await resultsController.configure(req, {}, nextMock) - expect(nextMock).toHaveBeenCalledWith(new Error('Request not found', { message: 'Request not found', status: 404 }), req, {}, nextMock) - }) - - it('should call next with a 500 error if the result processing errored', async () => { - asyncRequestApi.getRequestData = vi.fn().mockImplementation(() => { - throw new Error('Unexpected error', { message: 'Unexpected error', status: 500 }) - }) - - const nextMock = vi.fn() - await resultsController.configure(req, {}, nextMock) - expect(nextMock).toHaveBeenCalledWith(new Error('Unexpected error', { message: 'Unexpected error', status: 500 }), req, {}, nextMock) - }) - - it('should set the template to the errors template if the result has errors', async () => { - const mockResult = { hasErrors: () => true, isFailed: () => false, isComplete: () => true } - asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) - - await resultsController.configure(req, {}, () => {}) - expect(resultsController.template).toBe('results/errors') - }) - - it('should set the template to the no-errors template if the result has no errors', async () => { - const mockResult = { hasErrors: () => false, isFailed: () => false, isComplete: () => true } - asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) - - await resultsController.configure(req, {}, () => {}) - expect(resultsController.template).toBe('results/no-errors') - }) - - it('should set the template to the failedRequest template if the result is failed', async () => { - const mockResult = { isFailed: () => true, hasErrors: () => false, isComplete: () => true } - asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) - - await resultsController.configure(req, {}, () => {}) - expect(resultsController.template).toBe('results/failedRequest') - }) - - it('should redirect to the status page if the result is not complete', async () => { - const mockResult = { isFailed: () => true, hasErrors: () => false, isComplete: () => false } - asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) - - const res = { redirect: vi.fn() } - await resultsController.configure(req, res, () => {}) - expect(res.redirect).toHaveBeenCalledWith(`/status/${req.params.id}`) - }) - }) - describe('locals', () => { it('should set the result to the form options if the result is complete', async () => { - resultsController.result = { + const mockResult = { isComplete: () => true, - getParams: () => ('params'), + isFailed: () => false, + getParams: () => 'params', getErrorSummary: () => (['error summary']), getGeometries: () => ['geometries'], getColumns: () => (['columns']), @@ -95,11 +33,22 @@ describe('ResultsController', () => { getFields: () => (['fields']), getFieldMappings: () => ({ fields: 'geometries' }), hasErrors: () => false, - getPagination: () => 'pagination' + getPagination: () => 'pagination', + fetchResponseDetails: () => {} + } + const req = { + params: { id: 'test_id' }, + form: { + options: {} + } } const res = { redirect: vi.fn() } + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) + asyncRequestApi.g = vi.fn().mockResolvedValue(mockResult) + await resultsController.locals(req, res, () => {}) + expect(req.form.options.data).toBe(mockResult) expect(req.form.options.requestParams).toBe('params') expect(req.form.options.errorSummary).toStrictEqual(['error summary']) expect(req.form.options.columns).toStrictEqual(['columns']) @@ -110,15 +59,58 @@ describe('ResultsController', () => { }) }) - describe('noErrors', () => { - it('should return false if the result has errors', () => { - resultsController.result = { hasErrors: () => true } - expect(resultsController.noErrors()).toBe(false) + it("should call next with a 404 error if the result wasn't found", async () => { + asyncRequestApi.getRequestData = vi.fn().mockImplementation(() => { + throw new Error('Request not found', { message: 'Request not found', status: 404 }) }) - it('should return true if the result has no errors', () => { - resultsController.result = { hasErrors: () => false } - expect(resultsController.noErrors()).toBe(true) + const nextMock = vi.fn() + await resultsController.locals(req, {}, nextMock) + expect(nextMock).toHaveBeenCalledWith(new Error('Request not found', { message: 'Request not found', status: 404 }), req, {}, nextMock) + }) + + it('should call next with a 500 error if the result processing errored', async () => { + asyncRequestApi.getRequestData = vi.fn().mockImplementation(() => { + throw new Error('Unexpected error', { message: 'Unexpected error', status: 500 }) }) + + const nextMock = vi.fn() + await resultsController.locals(req, {}, nextMock) + expect(nextMock).toHaveBeenCalledWith(new Error('Unexpected error', { message: 'Unexpected error', status: 500 }), req, {}, nextMock) + }) + + it('should set the template to the errors template if the result has errors', async () => { + const mockResult = { hasErrors: () => true, isFailed: () => false, isComplete: () => true } + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) + + await resultsController.locals(req, {}, () => {}) + expect(req.form.options.template).toBe('results/errors') + expect(resultsController.noErrors(req)).toBe(false) + }) + + it('should set the template to the no-errors template if the result has no errors', async () => { + const mockResult = { hasErrors: () => false, isFailed: () => false, isComplete: () => true } + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) + + await resultsController.locals(req, {}, () => {}) + expect(req.form.options.template).toBe('results/no-errors') + expect(resultsController.noErrors(req)).toBe(true) + }) + + it('should set the template to the failedRequest template if the result is failed', async () => { + const mockResult = { isFailed: () => true, hasErrors: () => false, isComplete: () => true } + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) + + await resultsController.locals(req, {}, () => {}) + expect(req.form.options.template).toBe('results/failedRequest') + }) + + it('should redirect to the status page if the result is not complete', async () => { + const mockResult = { isFailed: () => true, hasErrors: () => false, isComplete: () => false } + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) + + const res = { redirect: vi.fn() } + await resultsController.locals(req, res, () => {}) + expect(res.redirect).toHaveBeenCalledWith(`/status/${req.params.id}`) }) }) diff --git a/test/unit/statusController.test.js b/test/unit/statusController.test.js index 8cc6157a..724bf091 100644 --- a/test/unit/statusController.test.js +++ b/test/unit/statusController.test.js @@ -15,14 +15,12 @@ describe('StatusController', () => { }) }) - describe('configure', () => { + describe('locals', () => { it('configure should make a request and attach the result of that request to the req.form.options object', async () => { const req = { params: { id: 'test_id' }, form: { - options: { - - } + options: {} } } const res = { render: vi.fn(), redirect: vi.fn() } @@ -31,28 +29,9 @@ describe('StatusController', () => { const mockResult = { response: { test: 'test' }, hasErrors: () => false } asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) - await statusController.configure(req, res, next) + await statusController.locals(req, res, next) expect(asyncRequestApi.getRequestData).toHaveBeenCalledWith(req.params.id) }) }) - - describe('locals', () => { - it('should attach the result of the request to the req.form.options.data object', async () => { - const req = { - form: { - options: {} - } - } - const res = {} - const next = vi.fn() - - const mockResult = { response: { test: 'test' }, hasErrors: () => false } - statusController.result = mockResult - - statusController.locals(req, res, next) - - expect(req.form.options.data).toBe(mockResult) - }) - }) }) diff --git a/test/unit/uploadController.test.js b/test/unit/uploadController.test.js index ce2d9a9b..e0a055e1 100644 --- a/test/unit/uploadController.test.js +++ b/test/unit/uploadController.test.js @@ -12,18 +12,6 @@ describe('UploadController', () => { uploadController = new UploadController(options) }) - it('resetValidationErrorMessage', ({ assert }) => { - uploadController.validationErrorMessage = 'Error message' - uploadController.resetValidationErrorMessage() - expect(uploadController.validationErrorMessage).toBe(undefined) - }) - - it('validationError', ({ assert }) => { - const errorObject = new Error('Test error') - uploadController.validationError('TestType', 'Test message', errorObject, {}) - expect(uploadController.validationErrorMessage).toBe('Test message') - }) - it('getBaseFormData', ({ assert }) => { const req = { sessionModel: { From d5c872bdd488087a56b76636f6154d47cb47d49e Mon Sep 17 00:00:00 2001 From: Chris Cundill Date: Tue, 28 May 2024 10:04:45 +0100 Subject: [PATCH 16/17] Removed commented, unused code --- test/acceptance/request_check.test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/acceptance/request_check.test.js b/test/acceptance/request_check.test.js index e2b12152..77b64936 100644 --- a/test/acceptance/request_check.test.js +++ b/test/acceptance/request_check.test.js @@ -15,10 +15,6 @@ import { uploadMethods } from '../PageObjectModels/uploadMethodPage' test.setTimeout(300000) -// test.beforeEach(async ({ page }, testInfo) => { -// test.setTimeout(Number("60,000")); -// }); - test.describe('Request Check', () => { test.describe('with javascript enabled', () => { test('request check of a @datafile', async ({ page }) => { From a3dfe83fad18b07958d5e241128b85b3474df9d9 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 28 May 2024 15:56:14 +0100 Subject: [PATCH 17/17] Update uploadFilePage.js remove debugging lines --- test/PageObjectModels/uploadFilePage.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/PageObjectModels/uploadFilePage.js b/test/PageObjectModels/uploadFilePage.js index fe97cac7..56935eaf 100644 --- a/test/PageObjectModels/uploadFilePage.js +++ b/test/PageObjectModels/uploadFilePage.js @@ -15,9 +15,6 @@ export default class UploadFilePage extends BasePage { async clickContinue (skipVerification) { await super.clickContinue() - await this.page.waitForTimeout(5000) - await this.page.screenshot({ path: 'playwright-report/data/screenshot.png', fullPage: true }) - console.log('URL here:', this.page.url()) return await super.verifyAndReturnPage(StatusPage, skipVerification) } }