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

[Security Solution] Prevent junit tranformation command from breaking a build step #165824

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

maximpn
Copy link
Contributor

@maximpn maximpn commented Sep 6, 2023

Summary

This PR implements a temporary fix to prevent yarn junit:merge command from breaking a build step by returning non zero code. yarn junit:merge fails in case if it can't find reports or folders it operates on weren't created beforehand.

@maximpn maximpn added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. ci:all-cypress-suites v8.11.0 labels Sep 6, 2023
@maximpn maximpn self-assigned this Sep 6, 2023
@maximpn maximpn marked this pull request as ready for review September 6, 2023 11:38
@maximpn maximpn requested a review from a team as a code owner September 6, 2023 11:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@maximpn
Copy link
Contributor Author

maximpn commented Sep 6, 2023

@elastic/kibana-operations Can we merge this PR? Osquery Cypress Tests fail due to missed API version and there is a PR addressing it. The other touched build steps pass.

@tomsonpl
Copy link
Contributor

tomsonpl commented Sep 6, 2023

@maximpn The osquery and defend workflows tests have been adjusted. We shouldn't be blocking you any more 👍

Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

I believe these can still cause failures:

https://buildkite.com/elastic/kibana-pull-request/builds/156752#018a6a28-0719-4cfa-858a-1e9b24ca0c54/445-449

The post-command hook is also expecting junit reports to exist. It looks like these can be fixed by setting export DISABLE_MISSING_TEST_REPORT_ERRORS=true

Reference: https://github.com/elastic/kibana/blob/main/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/failed_tests_reporter_cli.ts#L94-L97

@maximpn maximpn force-pushed the prevent-junit-merge-failure branch from 7708e65 to a58ba36 Compare September 6, 2023 14:26
@maximpn
Copy link
Contributor Author

maximpn commented Sep 6, 2023

@jbudz I rebased the PR. Threat Intelligence Cypress tests are broken and export DISABLE_MISSING_TEST_REPORT_ERRORS=true won't solve the issue as Cypress fails with ERROR Error: ENOENT: no such file or directory, open './cypress/e2e/**/*.cy.ts'. DISABLE_MISSING_TEST_REPORT_ERRORS=true will help prevent failure in case of missing reports but we expect to always have reports for failed tests. If it's missing it has to be fixed.

@PhilippeOberti should take care of Threat Intelligence Cypress tests during this week.

@jbudz jbudz merged commit 9f2f739 into elastic:main Sep 6, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 6, 2023
@maximpn maximpn deleted the prevent-junit-merge-failure branch September 6, 2023 14:47
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 6, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Serverless Security Investigations Cypress Tests #5 / Export timelines "before all" hook for "Exports custom timeline(s)" "before all" hook for "Exports custom timeline(s)"

Metrics [docs]

✅ unchanged

History

  • 💔 Build #156752 failed 7708e6539a5404002840d88d309c3b54901d3d04

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @maximpn

mistic added a commit that referenced this pull request Sep 7, 2023
…te (#165929)

## Summary

Fixes failed build steps when there are no tests to execute and hence no
reports.

## Details

Despite this [PR](#165824) fixes
tests failure when there are no reports but `yarn junit:merge` tries to
convert them to junit format the problem is partially stays as `Failed
Test Reporter` runs for every build step meaning also for successful
build steps. We don't need to handle failed test reports for successful
build steps. There are cases when there are no tests to run so Cypress
doesn't produce reports and nothing is converted to junit format so
`Failed Test Reporter` fails and causes a build step to fails. It could
be solved by defining an environment variable
`DISABLE_MISSING_TEST_REPORT_ERRORS=true` but we need to make sure
reports exist for failed tests. Taking this into account we can rely on
`BUILDKITE_COMMAND_EXIT_STATUS` to avoid running `Failed Test Reporter`
if the build step successed.

One may ask why don't skip `Upload Artifacts` too. We may need some
build artifacts for successful build steps.

---------

Co-authored-by: Tiago Costa <[email protected]>
banderror pushed a commit to banderror/kibana that referenced this pull request Sep 25, 2023
…te (elastic#165929)

## Summary

Fixes failed build steps when there are no tests to execute and hence no
reports.

## Details

Despite this [PR](elastic#165824) fixes
tests failure when there are no reports but `yarn junit:merge` tries to
convert them to junit format the problem is partially stays as `Failed
Test Reporter` runs for every build step meaning also for successful
build steps. We don't need to handle failed test reports for successful
build steps. There are cases when there are no tests to run so Cypress
doesn't produce reports and nothing is converted to junit format so
`Failed Test Reporter` fails and causes a build step to fails. It could
be solved by defining an environment variable
`DISABLE_MISSING_TEST_REPORT_ERRORS=true` but we need to make sure
reports exist for failed tests. Taking this into account we can rely on
`BUILDKITE_COMMAND_EXIT_STATUS` to avoid running `Failed Test Reporter`
if the build step successed.

One may ask why don't skip `Upload Artifacts` too. We may need some
build artifacts for successful build steps.

---------

Co-authored-by: Tiago Costa <[email protected]>
(cherry picked from commit 369d2fe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:all-cypress-suites release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants