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

connectors-ci: better handle unexpected failures #24569

Merged

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Mar 27, 2023

What

Fix the following pipeline behaviors:

  • Acceptance tests and integration tests were skipped when no unit test is defined. We want to skip acceptance tests and integration tests only when unit tests are failing, not when they are not declared.
  • When a failure outside of tests raised an exception it was caught by the test context but did not lead to explicit failure of the test run and creation of a test report.

How

  • Only skip acceptance and integration tests when unit tests fail
  • Wrap the run method of the abstract Step class: run now handles dagger Query error and calls the _run method of the step (which is overriden by child classes like AcceptanceTests, CodeFormatChecks etc.)
  • Declare a report to not be successful when no step rans, add failed and skipped steps to the test report.

@alafanechere alafanechere marked this pull request as ready for review March 27, 2023 12:41
@alafanechere alafanechere requested a review from a team March 27, 2023 16:00
Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

LGTM!

git_branch = self.test_report.connector_test_context.git_branch.replace("/", "_")
suffix = f"{connector_name}/{git_branch}/{connector_version}/{git_revision}.json"
local_report_path = Path(local_test_reports_path_root + suffix)
await local_report_path.parents[0].mkdir(parents=True, exist_ok=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think this function could benefit from a docstring and a few comments :)

@alafanechere alafanechere enabled auto-merge (squash) March 28, 2023 07:26
@alafanechere alafanechere merged commit 7b1930a into master Mar 28, 2023
@alafanechere alafanechere deleted the augustin/connectors-ci/better-handle-unexpected-failures branch March 28, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants