Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tests): move e2e tests for consultation-portal to their app #17008

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
**/*.log
**/tmp/
**/temp/
**/.next/

# Outputs
**/dist/
Expand Down
63 changes: 58 additions & 5 deletions .github/workflows/pullrequest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ jobs:

outputs:
TEST_CHUNKS: ${{ steps.test_projects.outputs.CHUNKS }}
E2E_CI_CHUNKS: ${{ steps.e2e_ci_projects.outputs.CHUNKS }}
E2E_CHUNKS: ${{ steps.e2e_projects.outputs.CHUNKS }}
E2E_CI_BUILD_ID: ${{ steps.e2e_ci_projects.outputs.BUILD_ID }}
E2E_BUILD_ID: ${{ steps.e2e_projects.outputs.BUILD_ID }}
LINT_CHUNKS: ${{ steps.lint_projects.outputs.CHUNKS }}
BUILD_CHUNKS: ${{ steps.build_projects.outputs.CHUNKS }}
Expand Down Expand Up @@ -141,13 +143,25 @@ jobs:
echo "CHUNKS={\"projects\":$CHUNKS}" >> "$GITHUB_OUTPUT"
fi

- name: Prepare e2e-ci targets
id: e2e_ci_projects
env:
CHUNK_SIZE: 1
run: |
set -euo pipefail
CHUNKS="$(./scripts/ci/generate-chunks.sh e2e-ci)"
if [[ "$CHUNKS" != "[]" ]]; then
echo "CHUNKS={\"projects\":$CHUNKS}" >> "$GITHUB_OUTPUT"
fi
echo BUILD_ID="$GITHUB_RUN_ID-$GITHUB_RUN_NUMBER-$(uuidgen)" >> "$GITHUB_OUTPUT"

- name: Prepare e2e targets
id: e2e_projects
env:
CHUNK_SIZE: 1
run: |
set -euo pipefail
CHUNKS="$(./scripts/ci/generate-chunks.sh e2e-ci)"
CHUNKS="$(./scripts/ci/generate-chunks.sh e2e)"
if [[ "$CHUNKS" != "[]" ]]; then
echo "CHUNKS={\"projects\":$CHUNKS}" >> "$GITHUB_OUTPUT"
fi
Expand Down Expand Up @@ -213,10 +227,10 @@ jobs:
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
docker-registry: 821090935708.dkr.ecr.eu-west-1.amazonaws.com/

e2e:
e2e-ci:
needs:
- prepare
if: needs.prepare.outputs.E2E_CHUNKS
if: needs.prepare.outputs.E2E_CI_CHUNKS
runs-on: ec2-runners
container:
image: public.ecr.aws/m3u4c4h9/island-is/actions-runner-public:latest
Expand All @@ -227,10 +241,10 @@ jobs:
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
API_MOCKS: 'true'
NODE_OPTIONS: --max-old-space-size=4096
E2E_BUILD_ID: '${{ needs.prepare.outputs.E2E_BUILD_ID }}-${{ github.run_attempt }}'
E2E_CI_BUILD_ID: '${{ needs.prepare.outputs.E2E_CI_BUILD_ID }}-${{ github.run_attempt }}'
strategy:
fail-fast: false
matrix: ${{ fromJson(needs.prepare.outputs.E2E_CHUNKS) }}
matrix: ${{ fromJson(needs.prepare.outputs.E2E_CI_CHUNKS) }}
steps:
- uses: actions/checkout@v4

Expand All @@ -252,6 +266,42 @@ jobs:
- name: Running e2e tests
run: ./scripts/ci/40_e2e.sh "${AFFECTED_PROJECT}"

e2e:
needs:
- prepare
if: needs.prepare.outputs.E2E_CHUNKS
runs-on: ec2-runners
container:
image: public.ecr.aws/m3u4c4h9/island-is/actions-runner-public:latest
timeout-minutes: 35
env:
AFFECTED_PROJECT: ${{ matrix.projects }}
NODE_OPTIONS: --max-old-space-size=4096
E2E_BUILD_ID: '${{ needs.prepare.outputs.E2E_BUILD_ID }}-${{ github.run_attempt }}'
strategy:
fail-fast: false
matrix: ${{ fromJson(needs.prepare.outputs.E2E_CHUNKS) }}
steps:
- uses: actions/checkout@v4

- uses: actions/setup-node@v4
with:
node-version-file: 'package.json'

- name: Setup yarn
run: corepack enable

- name: Get cache
id: get-cache
uses: ./.github/actions/get-cache
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
keys: ${{ needs.prepare.outputs.CACHE_KEYS }}
enable-cache: 'node_modules,generated-files'

- name: Running e2e tests
run: yarn e2e "${AFFECTED_PROJECT}"

linting-workspace:
needs:
- prepare
Expand Down Expand Up @@ -410,12 +460,15 @@ jobs:
- run-shellcheck
- formatting
- e2e
- e2e-ci
- build
steps:
- name: Check prepare success
run: '[[ ${{ needs.prepare.result }} == "success" ]] || exit 1'
- name: Check tests success
run: '[[ ${{ needs.tests.result }} != "failure" ]] || exit 1'
- name: Check e2e-ci success
run: '[[ ${{ needs.e2e-ci.result }} != "failure" ]] || exit 1'
- name: Check e2e success
run: '[[ ${{ needs.e2e.result }} != "failure" ]] || exit 1'
- name: Check linting success
Expand Down
87 changes: 87 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
ARG NODE_IMAGE_TAG=20.15.0-alpine3.20

# Stage 1: Install dependencies
FROM node:${NODE_IMAGE_TAG} AS dependencies

# Set working directory
WORKDIR /app

# Install Python3 and build tools
RUN apk add --no-cache python3 py3-pip make g++ && ln -sf /usr/bin/python3 /usr/bin/python

# Copy only package management files
COPY package.json yarn.lock ./
COPY .yarn/ .yarn
COPY .yarnrc.yml .yarnrc.yml
COPY ./apps/native/app/package.json ./apps/native/app/package.json

# Install dependencies
RUN --mount=type=cache,target=/app/.yarn/cache \
yarn install --immutable && yarn cache clean

# Stage 2: Final runtime image
FROM node:${NODE_IMAGE_TAG}

# Install Python (runtime requirement if needed)
RUN apk add --no-cache python3 py3-pip && ln -sf /usr/bin/python3 /usr/bin/python

# Enable Corepack and Yarn 3.2.3
RUN corepack enable && corepack prepare yarn@3.2.3 --activate

# Set working directory
WORKDIR /app

# Copy from dependencies stage
COPY --from=dependencies /app/node_modules ./node_modules
COPY --from=dependencies /app/yarn.lock ./yarn.lock
COPY --from=dependencies /app/.yarn/ ./.yarn
COPY --from=dependencies /app/.yarnrc.yml ./.yarnrc.yml

# Copy source code and necessary configuration files
COPY ./apps ./apps
COPY ./libs ./libs
COPY ./infra ./infra
COPY ./tools ./tools
COPY ./tsconfig.base.json ./
COPY ./tsconfig.shared.json ./
COPY ./package.json ./

Comment on lines +23 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add security best practices

Consider enhancing the security of the final image:

  1. Run as non-root user
  2. Add HEALTHCHECK
  3. Set proper file permissions
 # Copy source code and necessary configuration files
 COPY ./apps ./apps
 COPY ./libs ./libs
 COPY ./infra ./infra
 COPY ./tools ./tools           
 COPY ./tsconfig.base.json ./   
 COPY ./tsconfig.shared.json ./      
 COPY ./package.json ./          
+
+# Create non-root user
+RUN addgroup -S appgroup && adduser -S appuser -G appgroup
+RUN chown -R appuser:appgroup /app
+USER appuser
+
+# Add healthcheck
+HEALTHCHECK --interval=30s --timeout=3s \
+  CMD wget --no-verbose --tries=1 --spider http://localhost:4200/samradsgatt || exit 1

Committable suggestion skipped: line range outside the PR's diff.

# Verify Yarn version in runtime container
RUN yarn --version

# Default command to run the app
CMD ["yarn", "nx", "serve"]

# ARG NODE_IMAGE_TAG=20.15.0-alpine3.20
# FROM node:${NODE_IMAGE_TAG} AS build

# WORKDIR /app

# # Install build dependencies
# RUN apk add --no-cache \
# python3 \
# py3-pip \
# make \
# g++

# # Set Python symlink
# RUN ln -sf /usr/bin/python3 /usr/bin/python

# # Install dependencies
# COPY package.json yarn.lock ./
# COPY .yarn/ .yarn
# COPY .yarnrc.yml .yarnrc.yml
# COPY ./apps/native/app/package.json ./apps/native/app/package.json

# # Run yarn install
# RUN --mount=type=cache,target=/app/.yarn/cache \
# yarn install --immutable

# # Copy source code
# COPY . .

# # Verify installation
# RUN yarn nx --version

# # Set default entrypoint
# CMD ["yarn", "nx"]
18 changes: 18 additions & 0 deletions apps/consultation-portal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,24 @@ Login here https://island-is.awsapps.com/start#/ (Contact devops if you need acc
Copy env variables as instructed [here](https://docs.devland.is/technical-overview/devops/dockerizing#troubleshooting) (image arrows 1,2,3)
Paste env variables into terminal

## E2E Testing

### Quick Start

To run the E2E tests for the `consultation-portal` app:

```bash
# Install dependencies
yarn install && yarn codegen

# Start the server
yarn nx e2e consultation-portal
```
Comment on lines +74 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the Quick Start guide with additional setup details.

The current instructions might be insufficient for first-time users. Consider adding:

  1. Prerequisites (e.g., required environment variables)
  2. Whether the API needs to be running
  3. Brief explanation of what the tests verify
  4. Common issues and troubleshooting steps

Here's a suggested enhancement:

 ### Quick Start
 
+Prerequisites:
+- Ensure you have the required environment variables (see Development section above)
+- The API should be running locally (follow steps 1-5 in the Development section)
+
 To run the E2E tests for the `consultation-portal` app:
 
 ```bash
 # Install dependencies
 yarn install && yarn codegen
 
 # Start the server
 yarn nx e2e consultation-portal

+These tests verify both authenticated and unauthenticated user flows in the consultation portal.
+
+#### Troubleshooting
+
+If the tests fail, ensure:
+- All environment variables are properly set
+- The API is running and accessible
+- You have the necessary permissions


<!-- This is an auto-generated comment by CodeRabbit -->


### More Resources

For further details, refer to the [E2E Testing Library README](../../libs/testing/e2e/README.md).

## Project owner

- [Stjórnarráðið](https://www.stjornarradid.is)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import {
BrowserContext,
expect,
test,
icelandicAndNoPopupUrl,
urls,
session,
} from '@island.is/testing/e2e'

test.describe('Consultation portal authenticated', { tag: '@fast' }, () => {
let context: BrowserContext
test.use({ baseURL: urls.islandisBaseUrl })

test.beforeAll(async ({ browser }) => {
context = await session({
browser: browser,
storageState: 'consultation-auth.json',
idsLoginOn: {
nextAuth: { nextAuthRoot: `${urls.islandisBaseUrl}/samradsgatt` },
},
phoneNumber: '0102989',
homeUrl: `${urls.islandisBaseUrl}/samradsgatt`,
authTrigger: async (page) => {
await page.goto('/samradsgatt')
await page.getByTestId('menu-login-btn').click()
return `${urls.islandisBaseUrl}/samradsgatt`
},
})
})

test.afterAll(async () => {
if (context) {
await context.close()
}
})
Comment on lines +14 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and avoid hard-coded test data

The setup hooks are functional but could be improved in several areas:

  1. Add error handling in the auth trigger:
authTrigger: async (page) => {
  try {
    await page.goto('/samradsgatt')
    await page.getByTestId('menu-login-btn').click()
    return `${urls.islandisBaseUrl}/samradsgatt`
  } catch (error) {
    console.error('Auth trigger failed:', error)
    throw error
  }
}
  1. Move test data to a configuration file:
// test-config.ts
export const TEST_DATA = {
  phoneNumber: '0102989',
  storageState: 'consultation-auth.json',
  // ... other test constants
}
  1. Add timeout handling for the session setup


test('logged in user should be shown instead of login', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await expect(
page.getByRole('button', { name: 'Gervimaður Ameríku' }),
).toBeVisible()
await expect(page.getByTestId('menu-login-btn')).not.toBeVisible()

await page.close()
})

test('subscriptions page should show logged in state', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await page.getByTestId('subscriptions-btn').click()
await expect(page.getByTestId('subscriptions-title')).toBeVisible()
await expect(page.getByTestId('action-card')).toBeVisible()
await expect(
page.getByRole('button', {
name: 'Skrá mig inn',
}),
).not.toBeVisible()

await page.close()
})

test('my subscriptions page should show logged in state', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt/minaraskriftir'))
await expect(page.getByTestId('subscriptions-title')).toBeVisible()
await expect(page.getByTestId('action-card')).not.toBeVisible()

await page.close()
})

test('advices page should show logged in state', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await page.goto('/umsagnir')
await expect(page.getByTestId('actionCard')).not.toBeVisible()

await page.close()
})

test('logout button should be visible', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await page.getByRole('button', { name: 'Gervimaður Ameríku' }).click()
await expect(page.getByRole('button', { name: 'Útskrá' })).toBeVisible()

await page.close()
})

// Additional tests
test('user should be able to navigate to profile page', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await page.getByRole('button', { name: 'Gervimaður Ameríku' }).click()
await page.getByRole('link', { name: 'Mínar síður' }).click()
await expect(page.getByTestId('profile-page')).toBeVisible()

await page.close()
})

test('user should be able to update profile information', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt/minar-sidur'))

await page.getByTestId('edit-profile-btn').click()
await page.fill('input[name="name"]', 'Updated Name')
await page.getByTestId('save-profile-btn').click()
await expect(page.getByTestId('profile-name')).toHaveText('Updated Name')

await page.close()
})

test('user should be able to view notifications', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await page.getByRole('button', { name: 'Gervimaður Ameríku' }).click()
await page.getByRole('link', { name: 'Tilkynningar' }).click()
await expect(page.getByTestId('notifications-page')).toBeVisible()

await page.close()
})

test('user should be able to log out', async () => {
const page = await context.newPage()
await page.goto(icelandicAndNoPopupUrl('/samradsgatt'))

await page.getByRole('button', { name: 'Gervimaður Ameríku' }).click()
await page.getByRole('button', { name: 'Útskrá' }).click()
await expect(page.getByTestId('menu-login-btn')).toBeVisible()

await page.close()
})
})
Comment on lines +1 to +139
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the shell script results, I can now provide a final response since I have sufficient information about the test coverage across components.

Test coverage improvements needed for critical user flows

The current E2E test suite covers basic authenticated flows but lacks coverage for several critical components and scenarios:

  • Error handling components (Error404, Error500, CustomError) need test coverage
  • Form validation and user input components (DebouncedSearch, SearchAndSort) lack tests
  • UI feedback components (PageLoader, CardSkeleton) are untested
  • Navigation components (Menu, Breadcrumbs) need test coverage
  • Authentication edge cases in AuthProvider component are not verified

Suggested test additions:

  • Error page rendering and navigation
  • Search functionality with invalid/empty inputs
  • Loading states during async operations
  • Menu navigation and breadcrumb trails
  • Session handling edge cases
🔗 Analysis chain

Verify test coverage for critical user flows

The test suite covers essential authenticated user flows, but consider adding tests for:

  • Form validation errors
  • API error responses
  • Session timeout handling
  • Concurrent session management
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for missing test coverage in related components
rg "export|interface|type" apps/consultation-portal/components/ -A 2 | \
  while IFS= read -r line; do
    if [[ $line =~ "export" ]]; then
      component=$(echo "$line" | awk '{print $3}')
      echo "Checking test coverage for: $component"
      rg "$component" apps/consultation-portal/e2e/ -c || echo "No tests found"
    fi
  done

Length of output: 63773

Loading
Loading