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

Revert "feat(system-e2e): Move system-e2e to NX, v4" #14783

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ testem.log
/debug.log
*.log

# Playwright
test-results/
playwright-report/
tmp-sessions/

# System Files
.DS_Store
Thumbs.db
Expand Down
6 changes: 1 addition & 5 deletions .vscode/extensions.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
{
"recommendations": [
"esbenp.prettier-vscode",
"firsttris.vscode-jest-runner",
"ms-playwright.playwright"
]
"recommendations": ["esbenp.prettier-vscode", "firsttris.vscode-jest-runner"]
}
10 changes: 10 additions & 0 deletions apps/system-e2e/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"presets": [
"@nrwl/js/babel",
"next/babel",
"../../libs/shared/babel/web",
"@babel/preset-env",
"@babel/preset-typescript"
],
"plugins": ["@babel/plugin-proposal-class-properties"]
}
21 changes: 4 additions & 17 deletions apps/system-e2e/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,10 @@
{
"extends": ["plugin:playwright/recommended", "../../.eslintrc"],
"extends": "../../.eslintrc",
"rules": {},
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
},
{
"files": ["e2e/**/*.{ts,js,tsx,jsx}"],
"rules": {}
}
{ "files": ["*.ts", "*.tsx", "*.js", "*.jsx"], "rules": {} },
{ "files": ["*.ts", "*.tsx"], "rules": {} },
{ "files": ["*.js", "*.jsx"], "rules": {} }
]
}
3 changes: 3 additions & 0 deletions apps/system-e2e/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
test-results/
playwright-report/
tmp-sessions/
182 changes: 60 additions & 122 deletions apps/system-e2e/README.md

Large diffs are not rendered by default.

66 changes: 0 additions & 66 deletions apps/system-e2e/build.sh

This file was deleted.

10 changes: 0 additions & 10 deletions apps/system-e2e/e2e/example.spec.ts

This file was deleted.

18 changes: 7 additions & 11 deletions apps/system-e2e/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,24 @@ if [[ "$*" =~ --project ]]; then
fi

export TEST_PROJECT TEST_ENVIRONMENT TEST_TYPE TEST_RESULTS_S3 TEST_FILTER
export PATH="./node_modules/.bin:$PATH"

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a comment to explain the purpose of setting the DIR variable.

+ # Set the directory of the current script
DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
# Set the directory of the current script
DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"

DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"

echo "Current test environment: ${TEST_ENVIRONMENT}"
echo "Playwright args: $*"
if ! command -v playwright >/dev/null; then
echo "Can't find 'playwright'. Is it installed?"
echo "Your \$PATH might need updating"
echo " PATH=$PATH"
exit 1
fi
echo "Playwright version: $(playwright --version)"
echo "Playwright project: $TEST_PROJECT"
echo "Playwright version: $(yarn playwright --version)"

TEST_EXIT_CODE=0
playwright test -c . "$@" || TEST_EXIT_CODE=$?
yarn playwright test -c src --project="$TEST_PROJECT" "$@" || TEST_EXIT_CODE=$?

# Upload results
if [[ -n "$TEST_RESULTS_S3" ]]; then
zip -r -0 test-results playwright-report src/test-results
aws s3 cp test-results.zip "$TEST_RESULTS_S3"
fi
if [ "$TEST_EXIT_CODE" != "0" ]; then
node ./src/notifications/notify.*
yarn node "$DIR/src/notifications/notify.js"
fi

cat <<EOF
Expand All @@ -50,4 +46,4 @@ Additionally a web-based overview can be found at
https://www.tesults.com/digital-iceland/monorepo
EOF

exit "$TEST_EXIT_CODE"
exit $TEST_EXIT_CODE
12 changes: 12 additions & 0 deletions apps/system-e2e/global.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// global.d.ts
export {}

declare global {
namespace PlaywrightTest {
interface Matchers<R> {
toHaveCountGreaterThan(a: number): Promise<R>
toBeApplication(applicationType?: string): Promise<R>
}
}
}
export default {}
22 changes: 22 additions & 0 deletions apps/system-e2e/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "system-e2e",
"version": "1.0.0",
"main": "index.js",
"license": "MIT",
"dependencies": {
"@aws-sdk/client-ses": "^3.145.0",
"@formatjs/intl": "^1.8.1",
"@playwright/test": "^1.29.2",
"axios": "^0.21.4",
"imap-simple": "^5.1.0",
"mailparser": "^3.5.0",
"nodemailer": "^6.7.2",
"playwright-tesults-reporter": "^1.0.1",
"react": "^18.2.0",
"react-intl": "^6.3.0",
"slack-notify": "^2.0.3"
},
"volta": {
"node": "16.17.0"
}
}
39 changes: 0 additions & 39 deletions apps/system-e2e/playwright.config.ts

This file was deleted.

32 changes: 10 additions & 22 deletions apps/system-e2e/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
"build": {
"executor": "nx:run-commands",
"options": {
"commands": ["bash {projectRoot}/build.sh"],
"outputPath": "dist/{projectRoot}",
"commands": [
"esbuild --bundle $(find apps/system-e2e -name '*.ts' -not -path '*/node_modules/*') --outdir=dist/apps/system-e2e --tsconfig=apps/system-e2e/tsconfig.json --platform=node $(jq -r '.dependencies|keys[]|(\"--external:\"+.)' apps/system-e2e/package.json | xargs) --external:@nestjs/microservices --external:@nestjs/websockets --external:fsevents --external:class-transformer --external:canvas",
"cp apps/system-e2e/package.json dist/apps/system-e2e/"
],
"outputPath": "dist/apps/system-e2e",
"parallel": false
},
"configurations": {
Expand All @@ -21,34 +24,19 @@
"lint": {
"executor": "@nx/linter:eslint",
"options": {
"lintFilePatterns": ["{projectRoot}/**/*.{ts,tsx,js,jsx}"]
"lintFilePatterns": ["apps/system-e2e/**/*.{ts,tsx,js,jsx}"]
}
},
"smoke": {
"executor": "@nx/playwright:playwright",
"executor": "nx:run-commands",
"options": {
"config": "{projectRoot}/playwright.config.ts"
"command": "yarn playwright test apps/system-e2e/src/tests/*/smoke -c apps/system-e2e/src"
}
},
"acceptance": {
"executor": "@nx/playwright:playwright",
"options": {
"config": "{projectRoot}/playwright.config.ts",
"grep": "/acceptance/"
}
},
"e2e:all": {
"executor": "@nx/playwright:playwright",
"options": {
"config": "{projectRoot}/playwright.config.ts"
}
},
"e2e": {
"executor": "@nx/playwright:playwright",
"executor": "nx:run-commands",
"options": {
"config": "{projectRoot}/playwright.config.ts",
"grep": "e2e/example.spec.ts",
"skipInstall": true
"command": "yarn playwright test apps/system-e2e/src/tests -c apps/system-e2e/src"
}
},
"docker-playwright": {}
Expand Down
41 changes: 41 additions & 0 deletions apps/system-e2e/src/addons.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { expect, Locator, Page } from '@playwright/test'
import { sleep } from './support/utils'

expect.extend({
async toHaveCountGreaterThan(
received: Locator,
value: number,
options: { timeout: number; sleepTime: number } = {
timeout: 10000,
sleepTime: 100,
},
) {
const initialTime = Date.now()
let count = -1
while (count <= value) {
count = await received.count()
if (Date.now() > initialTime + options.timeout)
return { message: () => 'Timeout', pass: false }
await sleep(options.sleepTime)
}
return {
message: () => `Found ${count} elements`,
pass: true,
}
},
Comment on lines +5 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Recommendation

The toHaveCountGreaterThan matcher does not explicitly handle the case where the locator is not found. Additionally, the test files do not include scenarios where the locator might not be found. To ensure robustness, consider the following improvements:

  • Implementation Update: Modify the toHaveCountGreaterThan matcher to handle cases where the locator is not found.
  • Test Cases: Add test cases to cover scenarios where the locator might not be found.

Suggested Implementation Update

async toHaveCountGreaterThan(
  received: Locator,
  value: number,
  options: { timeout: number; sleepTime: number } = {
    timeout: 10000,
    sleepTime: 100,
  },
) {
  const initialTime = Date.now()
  let count = -1
  while (count <= value) {
    try {
      count = await received.count()
    } catch (error) {
      return { message: () => `Locator not found: ${error.message}`, pass: false }
    }
    if (Date.now() > initialTime + options.timeout)
      return { message: () => 'Timeout', pass: false }
    await sleep(options.sleepTime)
  }
  return {
    message: () => `Found ${count} elements`,
    pass: true,
  }
}

Suggested Test Cases

Add test cases in the relevant test files to cover scenarios where the locator might not be found.

  • Example Test Case:
test('should fail when locator is not found', async () => {
  const nonExistentLocator = page.locator('.non-existent-class')
  await expect(nonExistentLocator).toHaveCountGreaterThan(0)
})

Files to Update

  • apps/system-e2e/src/addons.ts
  • Relevant test files such as:
    • apps/system-e2e/src/tests/islandis/web/smoke/search.spec.ts
    • apps/system-e2e/src/tests/islandis/service-portal/smoke/sessions.spec.ts
    • apps/system-e2e/src/tests/islandis/web/smoke/homepage.spec.ts
    • apps/system-e2e/src/tests/islandis/admin-portal/smoke/endorsements.spec.ts
Analysis chain

Ensure the toHaveCountGreaterThan matcher handles edge cases, such as when the locator is not found.


Step 1: Review the implementation in addons.ts

The implementation of toHaveCountGreaterThan in addons.ts is as follows:

async toHaveCountGreaterThan(
  received: Locator,
  value: number,
  options: { timeout: number; sleepTime: number } = {
    timeout: 10000,
    sleepTime: 100,
  },
) {
  const initialTime = Date.now()
  let count = -1
  while (count <= value) {
    count = await received.count()
    if (Date.now() > initialTime + options.timeout)
      return { message: () => 'Timeout', pass: false }
    await sleep(options.sleepTime)
  }
  return {
    message: () => `Found ${count} elements`,
    pass: true,
  }
}

Step 2: Check the usage in test files

The search results show the usage of toHaveCountGreaterThan in several test files. We need to review these files to see if they include scenarios where the locator might not be found.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the `toHaveCountGreaterThan` matcher handles edge cases.

# Test: Search for the `toHaveCountGreaterThan` matcher implementation and usage. Expect: Proper handling of edge cases.
rg --type ts 'toHaveCountGreaterThan'

Length of output: 1157



Script:

#!/bin/bash
# Description: Review the test files for edge case handling where the locator might not be found.

# List the test files that use `toHaveCountGreaterThan`
files=$(rg --type ts 'toHaveCountGreaterThan' --files-with-matches)

# Display the relevant lines from each test file
for file in $files; do
  echo "File: $file"
  rg --type ts 'toHaveCountGreaterThan' $file
  echo ""
done

Length of output: 2449

async toBeApplication(received: string | Page, applicationType = '\\w+') {
const url: string = typeof received == 'string' ? received : received.url()
const protocol = 'https?://'
const host = '[^/]+'
const applicationId = '(/(\\w|-)*)?'
const applicationRegExp = new RegExp(
`^${protocol}${host}/umsoknir/${applicationType}${applicationId}$`,
)
const pass = applicationRegExp.test(url)
const message = () =>
`Current page is ${pass ? '' : '*not* '}an application
Pattern ${applicationRegExp}
URL is ${url}`
return { message, pass }
},
})
Loading
Loading