Skip to content

Regression tests: display stdout/stderr even if there was an exception before the report could be created. #38008

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 7, 2024
Merged

Regression tests: display stdout/stderr even if there was an exception before the report could be created. #38008

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8ce766
Regression tests: display stdout/stderr even if there was an exception
clnoll May 7, 2024
7068f11
CR suggestion
clnoll May 7, 2024
939b8b2
formatting
clnoll May 7, 2024
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
13 changes: 9 additions & 4 deletions airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import datetime
import os
import time
import traceback
from abc import ABC, abstractmethod
from functools import cached_property
from pathlib import Path
Expand All @@ -17,7 +18,7 @@
import semver
import yaml # type: ignore
from dagger import Container, Directory
from pipelines import hacks
from pipelines import hacks, main_logger
from pipelines.airbyte_ci.connectors.consts import CONNECTOR_TEST_STEP_ID
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.airbyte_ci.steps.docker import SimpleDockerStep
Expand Down Expand Up @@ -416,12 +417,16 @@ async def _run(self, connector_under_test_container: Container) -> StepResult:
container = container.with_(hacks.never_fail_exec(self.regression_tests_command()))
regression_tests_artifacts_dir = str(self.regression_tests_artifacts_dir)
path_to_report = f"{regression_tests_artifacts_dir}/session_{self.run_id}/report.html"
await container.file(path_to_report).export(path_to_report)

exit_code, stdout, stderr = await get_exec_result(container)

with open(path_to_report, "r") as fp:
regression_test_report = fp.read()
try:
await container.file(path_to_report).export(path_to_report)
with open(path_to_report, "r") as fp:
regression_test_report = fp.read()
except Exception as exc:
main_logger.exception(f"Could not export or read report due to exception. traceback={traceback.format_exc()}", exc_info=exc)
regression_test_report = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Just summarizing my understanding of the flow.

  • get_exec_result actually executes the command in the container
  • If there's an ExecError during the execution of this command we get stdout and stderr from the exception object
  • If an ExecError was thrown (and handled) executing container.file() will likely fail as we're trying to get a file that does not exist.

Instead of excepting a broad exception I'd be in favor of being more specific:

if "report.html" not in await container.directory(f"{regression_tests_artifacts_dir}/session_{self.run_id}").entries():
   main_logger.exception("The report file was not generated, an unhandled error likely happened during regression test execution, please check the step stderr and stdout for more details")
else:
  await container.file(path_to_report).export(path_to_report)
  with open(path_to_report, "r") as fp:
    regression_test_report = fp.read()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the suggestion, updated.


return StepResult(
step=self,
Expand Down
Loading