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

Test phase does not report error with exit 1 #2409

Closed
4 tasks done
cfbo opened this issue Nov 24, 2021 · 9 comments
Closed
4 tasks done

Test phase does not report error with exit 1 #2409

cfbo opened this issue Nov 24, 2021 · 9 comments
Labels
archived This issue has been locked. bug Something isn't working e2e-cypress-tests Cypress/E2E test step issue

Comments

@cfbo
Copy link

cfbo commented Nov 24, 2021

Before opening, please confirm:

App Id

d2oeatkcx6u4xt

Region

eu-central-1

Amplify Console feature

Custom builds, E2E tests

Describe the bug

I am reporting the bug as suggested by josef in Discord https://discord.com/channels/705853757799399426/707328986077855836/913090540998909992

image

The testing phase doesn't pick up an exit 1 code as an error

Expected behavior

exit 1 should make the build fail

Reproduction steps

Use the following

    test:
      commands:
        - exit 1

Build Settings

No response

Additional information

To give a bit of context, what I was trying to do is to run jest tests in a lambda as part of the amplify build.

After many attempts I have got the following solution that works when all tests are succeeding, but when there are test failures, jest exits with exit code 1 and that value doesn't make the build fail.

test:
  artifacts:
    baseDirectory: ./
    configFilePath: '**/mochawesome.json'
    files:
      - '**/*.mp4'
  phases:
    preTest:
      commands:
        - 'cd ./amplify/backend/function/nodeLambda/src'
        - 'npm install'
        - 'cd -'
    test:
      commands:
        - 'npm --prefix ./amplify/backend/function/nodeLambda/src run test'
        - TEST_EXIT_CODE=$?
        - exit $TEST_EXIT_CODE

Issues that are partially related:

Please read the discord because there are a lot information and attempts reported around that point: https://discord.com/channels/705853757799399426/707328986077855836/913100295079329904

@cfbo
Copy link
Author

cfbo commented Nov 25, 2021

I will add another point here because it's sort of related. If someone gives some sort of feedback on this I can also create a separate ticket for this. The question is: if I have an amplify.yml without test section with connected frontend branches i.e. cloud build associated to git commits, will the amplify build be able to start running tests once I add the test section in the yml? From what I see the answer is no.
The point is: if you have existing envs and branches connected to amplify for automated build, when you add the test part to the yml, no tests are being run.
https://discord.com/channels/705853757799399426/707328986077855836/913364290889728000
https://discord.com/channels/705853757799399426/707328986077855836/913120537289564190
Can anyone confirm whether this is an expected behavior or not? If it is, I would suggest that it would be useful for amplify to pickup changes to the "detected frameworks" logic more dynamically.

More in detail, after adding that change:
image
no tests were run:
image
while of course I have tests to run which are working when invoked locally using the same command.
Info about that particular environment:
appid: d3i83n9mlag28r
region: eu-central-1
feature/REG-348-aws-lambda-testing
build 1

@siegerts siegerts added the e2e-cypress-tests Cypress/E2E test step issue label Nov 25, 2021
@cfbo
Copy link
Author

cfbo commented Dec 20, 2021

Any feedback on this?

@shubhamsharma04
Copy link

Hi @cfbo , sorry for the late reply. An app for which tests fail shouldn't be deployed, and adding test section to an existing app adds the test step in the workflow. I was unable to reproduce both the scenarios, I am still investigating why this issue happened with you.

@ghost
Copy link

ghost commented Feb 1, 2022

I will add another point here because it's sort of related. If someone gives some sort of feedback on this I can also create a separate ticket for this. The question is: if I have an amplify.yml without test section with connected frontend branches i.e. cloud build associated to git commits, will the amplify build be able to start running tests once I add the test section in the yml? From what I see the answer is no. The point is: if you have existing envs and branches connected to amplify for automated build, when you add the test part to the yml, no tests are being run. https://discord.com/channels/705853757799399426/707328986077855836/913364290889728000 https://discord.com/channels/705853757799399426/707328986077855836/913120537289564190 Can anyone confirm whether this is an expected behavior or not? If it is, I would suggest that it would be useful for amplify to pickup changes to the "detected frameworks" logic more dynamically.

The tests should run on the next deployment. Is this not happening after you've modified the amplify.yml and re-deployed?

@ghost
Copy link

ghost commented Feb 1, 2022

Also can you try installing the pm2 package in the preTest phase and then add the npx pm2 kill script to the postTest phase as shown in the default Cypress build scripts to try and get the pipeline to fail upon test failure?

@ghost ghost self-assigned this Feb 1, 2022
@ghost ghost added the bug Something isn't working label Feb 1, 2022
@mmirus
Copy link

mmirus commented Feb 7, 2022

I also hit this problem (failed tests didn't stop deployment). Using pm2 as suggested above fixed it (failed tests now stop deployment). Thanks for the recommendation.

Can you help me understand what pm2 is doing here and why it's needed?


Here's what I had that was not working, for a standard create-react-app project that runs tests with jest:

version: 1
backend:
  phases:
    build:
      commands:
        - '# Execute Amplify CLI with the helper script'
        - amplifyPush --simple
frontend:
  phases:
    preBuild:
      commands:
        - npm install
    build:
      commands:
        - npm run build
  artifacts:
    baseDirectory: build
    files:
      - '**/*'
  cache:
    paths:
      - node_modules/**/*
test:
  phases:
    preTest:
      commands:
        - npm ci
    test:
      commands:
        - CI=true npm test -- --outputFile jestOutput.json --json
  artifacts:
    baseDirectory: .
    configFilePath: 'jestOutput.json'
    files:
      - '**/__snapshots__/*.snap'
  cache:
    paths:
      - node_modules/**/*

Expected results

Failed tests stop deployment.

Actual results

Tests fail, but Amplify doesn't detect it and stop deploying.

			         # Starting phase: preTest
                                 # Executing command: npm ci
2022-02-05T18:53:31.307Z [WARNING]: npm
2022-02-05T18:53:31.307Z [WARNING]: WARN prepare removing existing node_modules/ before installation
2022-02-05T18:53:45.770Z [INFO]: > [email protected] postinstall /codebuild/output/src790932801/src/redacted-project/node_modules/core-js
                                 > node -e "try{require('./postinstall')}catch(e){}"
2022-02-05T18:53:45.817Z [INFO]: �[96mThank you for using core-js (�[94m https://github.com/zloirock/core-js �[96m) for polyfilling JavaScript standard library!�[0m
                                 �[96mThe project needs your help! Please consider supporting of core-js:�[0m
                                 �[96m>�[94m https://opencollective.com/core-js �[0m
                                 �[96m>�[94m https://patreon.com/zloirock �[0m
                                 �[96m>�[94m https://paypal.me/zloirock �[0m
                                 �[96m>�[94m bitcoin: bc1qlea7544qtsmj2rayg0lthvza9fau63ux0fstcz �[0m
                                 �[96mAlso, the author of core-js (�[94m https://github.com/zloirock �[96m) is looking for a good job -)�[0m
2022-02-05T18:53:45.967Z [INFO]: > [email protected] postinstall /codebuild/output/src790932801/src/redacted-project/node_modules/core-js-pure
                                 > node -e "try{require('./postinstall')}catch(e){}"
2022-02-05T18:53:46.172Z [INFO]: > [email protected] postinstall /codebuild/output/src790932801/src/redacted-project/node_modules/msw
                                 > node -e "try{require('./config/scripts/postinstall')}catch(e){}"
2022-02-05T18:53:46.226Z [INFO]: added 1317 packages in 14.934s
2022-02-05T18:53:46.242Z [INFO]: # Completed phase: preTest
                                 # Starting phase: test
2022-02-05T18:53:46.243Z [INFO]: # Executing command: CI=true npm test -- --outputFile jestOutput.json --json
2022-02-05T18:53:46.414Z [INFO]: > [email protected] test /codebuild/output/src790932801/src/redacted-project
                                 > react-scripts test --testURL https://my.redacted.pizza "--outputFile" "jestOutput.json" "--json"
2022-02-05T18:53:49.297Z [WARNING]: PASS src/utils/getJwt.test.ts
2022-02-05T18:53:49.565Z [WARNING]: PASS src/utils/redactedBaseUrl.test.ts
2022-02-05T18:53:49.824Z [WARNING]: PASS src/context/AuthProvider.test.tsx
2022-02-05T18:53:49.892Z [WARNING]: PASS src/pages/App/App.test.tsx
# ✅ Jest fails test:
2022-02-05T18:53:50.051Z [WARNING]: FAIL src/utils/redactedEnvironment.test.ts 
2022-02-05T18:53:50.052Z [WARNING]: ● redactedEnvironment › should return the correct environment for the given domain
                                    expect(received).toBe(expected) // Object.is equality
                                    Expected: "development"
                                    Received: undefined
                                    3 | describe('redactedEnvironment', () => {
                                    4 |   test('should return the correct environment for the given domain', () => {
                                    > 5 |     expect(redactedEnvironment('redacted.pizzaaaaaaa')).toBe('development');
                                    |                                                     ^
                                    6 |     expect(redactedEnvironment('redacted.xyz')).toBe('integration');
                                    7 |     expect(redactedEnvironment('redacted.com')).toBe('staging');
                                    8 |     expect(redactedEnvironment('redacted.com')).toBe('production');
                                    at Object.<anonymous> (src/utils/redactedEnvironment.test.ts:5:53)
2022-02-05T18:53:50.116Z [WARNING]: PASS src/utils/redactedDomain.test.ts
2022-02-05T18:53:50.192Z [WARNING]: PASS src/context/GraphQlProvider.test.tsx
2022-02-05T18:53:50.211Z [WARNING]: 
2022-02-05T18:53:50.212Z [WARNING]: Test Suites: 1 failed, 6 passed, 7 total
                                    Tests:       1 failed, 14 passed, 15 total
                                    Snapshots:   0 total
                                    Time:        3.025 s
                                    Ran all test suites.
2022-02-05T18:53:50.213Z [WARNING]: Test results written to: jestOutput.json
2022-02-05T18:53:50.301Z [WARNING]: npm
# ✅ Amplify notes that the tests failed (I believe this is Amplify output, at least):
2022-02-05T18:53:50.302Z [WARNING]: ERR! Test failed.  See above for more details. 
2022-02-05T18:53:50.304Z [INFO]: # Completed phase: test
2022-02-05T18:53:50.305Z [INFO]: ## Tests completed successfully 
# 🤡 Wait, what? 👆
2022-02-05T18:53:50.307Z [INFO]: # Starting test artifact upload process...
2022-02-05T18:53:54.256Z [INFO]: # Uploading build artifact 'testConfig.json'...
2022-02-05T18:53:54.261Z [INFO]: # Build artifact is: 0MB
2022-02-05T18:53:54.262Z [INFO]: # Uploading build artifact 'testArtifacts.zip'...
2022-02-05T18:53:54.450Z [INFO]: # Test artifact upload completed
2022-02-05T18:53:54.453Z [INFO]: No custom headers found.
2022-02-05T18:53:54.469Z [INFO]: # Starting build artifact upload process...
2022-02-05T18:53:54.600Z [INFO]: # Build artifact is: 1MB
2022-02-05T18:53:54.601Z [INFO]: # Uploading build artifact '__artifacts.zip'...
2022-02-05T18:53:54.608Z [INFO]: # Build artifact is: 1MB
2022-02-05T18:53:54.608Z [INFO]: # Uploading build artifact '__artifactsHash.zip'...
2022-02-05T18:53:54.758Z [INFO]: # Build artifact upload completed
2022-02-05T18:53:54.759Z [INFO]: # Starting environment caching...
2022-02-05T18:53:54.759Z [INFO]: # Uploading environment cache artifact...
2022-02-05T18:53:54.851Z [INFO]: # Environment caching completed
Terminating logging...
2022-02-05T18:51:34.413Z [INFO]: Git SSH Key acquired
2022-02-05T18:51:34.489Z [INFO]: # Cloning repository: [email protected]:redacted/redacted-project.git
2022-02-05T18:51:34.983Z [INFO]: Agent pid 135
2022-02-05T18:51:35.017Z [INFO]: Identity added: /root/.ssh/git_rsa (/root/.ssh/git_rsa)
2022-02-05T18:51:35.483Z [INFO]: Cloning into 'redacted-project'...
2022-02-05T18:51:36.178Z [INFO]: Warning: Permanently added the ECDSA host key for IP address '192.30.255.113' to the list of known hosts.
2022-02-05T18:51:37.872Z [INFO]: # Switching to commit: 10a90f59848847e80fb6f67c13c03300810c50ef
2022-02-05T18:51:37.968Z [INFO]: Agent pid 148
2022-02-05T18:51:37.968Z [INFO]: Identity added: /root/.ssh/git_rsa (/root/.ssh/git_rsa)
                                 Note: switching to '10a90f59848847e80fb6f67c13c03300810c50ef'.
                                 You are in 'detached HEAD' state. You can look around, make experimental
                                 changes and commit them, and you can discard any commits you make in this
                                 state without impacting any branches by switching back to a branch.
                                 If you want to create a new branch to retain commits you create, you may
                                 do so (now or later) by using -c with the switch command. Example:
                                 git switch -c <new-branch-name>
                                 Or undo this operation with:
                                 git switch -
                                 Turn off this advice by setting config variable advice.detachedHead to false
                                 HEAD is now at 10a90f5 Verify failing test stops build now
2022-02-05T18:51:37.997Z [INFO]: Successfully cleaned up Git credentials
2022-02-05T18:51:37.997Z [INFO]: # Checking for Git submodules at: /codebuild/output/src790932801/src/redacted-project/.gitmodules
2022-02-05T18:51:38.019Z [INFO]: # Retrieving environment cache...
2022-02-05T18:51:38.076Z [INFO]: # Retrieved environment cache
2022-02-05T18:51:38.076Z [INFO]: ---- Setting Up SSM Secrets ----
2022-02-05T18:51:38.077Z [INFO]: SSM params {"Path":"/amplify/ddg88mx4nrct6/develop/","WithDecryption":true}
2022-02-05T18:51:38.152Z [WARNING]: ! Unable to patch packages...
2022-02-05T18:51:38.161Z [INFO]: # Retrieving cache...
2022-02-05T18:51:42.485Z [INFO]: # Extracting cache...
2022-02-05T18:51:46.322Z [INFO]: # Extraction completed
                                 # Starting phase: preTest
                                 # Executing command: npm ci
2022-02-05T18:53:31.307Z [WARNING]: npm
2022-02-05T18:53:31.307Z [WARNING]: WARN prepare removing existing node_modules/ before installation
2022-02-05T18:53:45.770Z [INFO]: > [email protected] postinstall /codebuild/output/src790932801/src/redacted-project/node_modules/core-js
                                 > node -e "try{require('./postinstall')}catch(e){}"
2022-02-05T18:53:45.817Z [INFO]: �[96mThank you for using core-js (�[94m https://github.com/zloirock/core-js �[96m) for polyfilling JavaScript standard library!�[0m
                                 �[96mThe project needs your help! Please consider supporting of core-js:�[0m
                                 �[96m>�[94m https://opencollective.com/core-js �[0m
                                 �[96m>�[94m https://patreon.com/zloirock �[0m
                                 �[96m>�[94m https://paypal.me/zloirock �[0m
                                 �[96m>�[94m bitcoin: bc1qlea7544qtsmj2rayg0lthvza9fau63ux0fstcz �[0m
                                 �[96mAlso, the author of core-js (�[94m https://github.com/zloirock �[96m) is looking for a good job -)�[0m
2022-02-05T18:53:45.967Z [INFO]: > [email protected] postinstall /codebuild/output/src790932801/src/redacted-project/node_modules/core-js-pure
                                 > node -e "try{require('./postinstall')}catch(e){}"
2022-02-05T18:53:46.172Z [INFO]: > [email protected] postinstall /codebuild/output/src790932801/src/redacted-project/node_modules/msw
                                 > node -e "try{require('./config/scripts/postinstall')}catch(e){}"
2022-02-05T18:53:46.226Z [INFO]: added 1317 packages in 14.934s
2022-02-05T18:53:46.242Z [INFO]: # Completed phase: preTest
                                 # Starting phase: test
2022-02-05T18:53:46.243Z [INFO]: # Executing command: CI=true npm test -- --outputFile jestOutput.json --json
2022-02-05T18:53:46.414Z [INFO]: > [email protected] test /codebuild/output/src790932801/src/redacted-project
                                 > react-scripts test --testURL https://my.redacted.pizza "--outputFile" "jestOutput.json" "--json"
2022-02-05T18:53:49.297Z [WARNING]: PASS src/utils/getJwt.test.ts
2022-02-05T18:53:49.565Z [WARNING]: PASS src/utils/redactedBaseUrl.test.ts
2022-02-05T18:53:49.824Z [WARNING]: PASS src/context/AuthProvider.test.tsx
2022-02-05T18:53:49.892Z [WARNING]: PASS src/pages/App/App.test.tsx
2022-02-05T18:53:50.051Z [WARNING]: FAIL src/utils/redactedEnvironment.test.ts
2022-02-05T18:53:50.052Z [WARNING]: ● redactedEnvironment › should return the correct environment for the given domain
                                    expect(received).toBe(expected) // Object.is equality
                                    Expected: "development"
                                    Received: undefined
                                    3 | describe('redactedEnvironment', () => {
                                    4 |   test('should return the correct environment for the given domain', () => {
                                    > 5 |     expect(redactedEnvironment('redacted.pizzaaaaaaa')).toBe('development');
                                    |                                                     ^
                                    6 |     expect(redactedEnvironment('redacted.xyz')).toBe('integration');
                                    7 |     expect(redactedEnvironment('redacted.com')).toBe('staging');
                                    8 |     expect(redactedEnvironment('redacted.com')).toBe('production');
                                    at Object.<anonymous> (src/utils/redactedEnvironment.test.ts:5:53)
2022-02-05T18:53:50.116Z [WARNING]: PASS src/utils/redactedDomain.test.ts
2022-02-05T18:53:50.192Z [WARNING]: PASS src/context/GraphQlProvider.test.tsx
2022-02-05T18:53:50.211Z [WARNING]: 
2022-02-05T18:53:50.212Z [WARNING]: Test Suites: 1 failed, 6 passed, 7 total
                                    Tests:       1 failed, 14 passed, 15 total
                                    Snapshots:   0 total
                                    Time:        3.025 s
                                    Ran all test suites.
2022-02-05T18:53:50.213Z [WARNING]: Test results written to: jestOutput.json
2022-02-05T18:53:50.301Z [WARNING]: npm
2022-02-05T18:53:50.302Z [WARNING]: ERR! Test failed.  See above for more details.
2022-02-05T18:53:50.304Z [INFO]: # Completed phase: test
2022-02-05T18:53:50.305Z [INFO]: ## Tests completed successfully
2022-02-05T18:53:50.307Z [INFO]: # Starting test artifact upload process...
2022-02-05T18:53:54.256Z [INFO]: # Uploading build artifact 'testConfig.json'...
2022-02-05T18:53:54.261Z [INFO]: # Build artifact is: 0MB
2022-02-05T18:53:54.262Z [INFO]: # Uploading build artifact 'testArtifacts.zip'...
2022-02-05T18:53:54.450Z [INFO]: # Test artifact upload completed
2022-02-05T18:53:54.453Z [INFO]: No custom headers found.
2022-02-05T18:53:54.469Z [INFO]: # Starting build artifact upload process...
2022-02-05T18:53:54.600Z [INFO]: # Build artifact is: 1MB
2022-02-05T18:53:54.601Z [INFO]: # Uploading build artifact '__artifacts.zip'...
2022-02-05T18:53:54.608Z [INFO]: # Build artifact is: 1MB
2022-02-05T18:53:54.608Z [INFO]: # Uploading build artifact '__artifactsHash.zip'...
2022-02-05T18:53:54.758Z [INFO]: # Build artifact upload completed
2022-02-05T18:53:54.759Z [INFO]: # Starting environment caching...
2022-02-05T18:53:54.759Z [INFO]: # Uploading environment cache artifact...
2022-02-05T18:53:54.851Z [INFO]: # Environment caching completed
Terminating logging...

Notes:

  1. I'm not sure why the tests are run twice
  2. Amplify doesn't seem to register this step in this part of the UI:
    image
  3. Amplify shows this success message above the log output in the "Test" tab:
    image

With pm2 added

As suggested, I've added pm2, and this version works as expected:

version: 1
backend:
  phases:
    build:
      commands:
        - '# Execute Amplify CLI with the helper script'
        - amplifyPush --simple
frontend:
  phases:
    preBuild:
      commands:
        - npm install
    build:
      commands:
        - npm run build
  artifacts:
    baseDirectory: build
    files:
      - '**/*'
  cache:
    paths:
      - node_modules/**/*
test:
  phases:
    preTest:
      commands:
        - npm ci
        - npm install pm2
    test:
      commands:
        - CI=true npm test -- --outputFile jestOutput.json --json
    postTest:
      commands:
        - npx pm2 kill
  artifacts:
    baseDirectory: .
    configFilePath: 'jestOutput.json'
    files:
      - '**/__snapshots__/*.snap'
  cache:
    paths:
      - node_modules/**/*

I wonder if this problem is why I've seen the test added to the build command in a lot of guides online, rather than in a proper test section. I haven't personally tried this, but it's like so:

# ...
frontend:
  phases:
    preBuild:
      commands:
        - npm install
    build:
      commands:
        - npm test && npm run build # 👈 this will prevent the build from running if the tests fail
  artifacts:
    baseDirectory: build
    files:
      - '**/*'
  cache:
    paths:
      - node_modules/**/*

Though it could also be the fact that the artifact config requirements make it difficult to set up the test section.

@ghost
Copy link

ghost commented Nov 1, 2022

Amplify Hosting only supports testing with Cypress at this time (see here). Please follow the feature request to support testing with the Jest framework on issue #464.

@ghost ghost closed this as completed Nov 1, 2022
@github-actions
Copy link

github-actions bot commented Nov 1, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions
Copy link

github-actions bot commented Nov 1, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot added the archived This issue has been locked. label Nov 1, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived This issue has been locked. bug Something isn't working e2e-cypress-tests Cypress/E2E test step issue
Projects
None yet
Development

No branches or pull requests

4 participants