-
Notifications
You must be signed in to change notification settings - Fork 371
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
Report test results for multiple suites #2747
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ | |
from lisa import schema # pylint: disable=E0401 | ||
from lisa.messages import ( # pylint: disable=E0401 | ||
MessageBase, | ||
TestResultMessageBase, | ||
TestResultMessage, | ||
) | ||
|
||
|
||
|
@@ -48,8 +48,11 @@ def type_schema(cls) -> Type[schema.TypedSchema]: | |
return AgentJUnitSchema | ||
|
||
def _received_message(self, message: MessageBase) -> None: | ||
if isinstance(message, TestResultMessageBase): | ||
if isinstance(message, TestResultMessage) and message.type != "AgentTestResultMessage": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AgentTestResultMessages are the messages generated for our test suites. They will include the name of each suite. On the next line I change the name of other TestResultMessages to "Setup". Those are the messages generated by LISA and include the creation, setup and deletion of the test VMs. I chose "Setup" because they are similar to the current setup logs in DCR. |
||
message.suite_full_name = "_Setup_" | ||
message.suite_name = message.suite_full_name | ||
image = message.information.get('image') | ||
if image is not None: | ||
message.name = image | ||
message.full_name = image | ||
message.name = message.full_name | ||
super()._received_message(message) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
from pathlib import Path | ||
from typing import Any, Dict, List, Type | ||
|
||
from tests_e2e.scenarios.lib.agent_test import AgentTest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed "scenarios" from the source code tree. Now "tests" and "subtests" are directly under "tests_e2e". The previous "scenarios/lib" is now "tests/lib". |
||
from tests_e2e.tests.lib.agent_test import AgentTest | ||
|
||
|
||
class TestSuiteDescription(object): | ||
|
@@ -39,18 +39,18 @@ def __init__(self, test_source_directory: Path): | |
""" | ||
The test_source_directory parameter must be the root directory of the end-to-end tests (".../WALinuxAgent/tests_e2e") | ||
""" | ||
self._root: Path = test_source_directory/"scenarios" | ||
self._root: Path = test_source_directory | ||
|
||
def load(self, test_suites: str) -> List[TestSuiteDescription]: | ||
""" | ||
Loads the specified 'test_suites', which are given as a string of comma-separated suite names or a JSON description | ||
of a single test_suite. | ||
|
||
When given as a comma-separated list, each item must correspond to the name of the JSON files describing s suite (those | ||
files are located under the .../WALinuxAgent/tests_e2e/scenarios/testsuites directory). For example, | ||
if test_suites == "agent_bvt, fast-track" then this method will load files agent_bvt.json and fast-track.json. | ||
files are located under the .../WALinuxAgent/tests_e2e/test_suites directory). For example, if test_suites == "agent_bvt, fast-track" | ||
then this method will load files agent_bvt.json and fast-track.json. | ||
|
||
When given as a JSON string, the value must correspond to the description a single test suite, for example | ||
When given as a JSON string, the value must correspond to the description a single test suite, for example | ||
|
||
{ | ||
"name": "AgentBvt", | ||
|
@@ -69,7 +69,7 @@ def load(self, test_suites: str) -> List[TestSuiteDescription]: | |
pass | ||
|
||
# Else, it should be a comma-separated list of description files | ||
description_files: List[Path] = [self._root/"testsuites"/f"{t.strip()}.json" for t in test_suites.split(',')] | ||
description_files: List[Path] = [self._root/"test_suites"/f"{t.strip()}.json" for t in test_suites.split(',')] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added an "_" to the name |
||
return [self._load_test_suite(AgentTestLoader._load_file(s)) for s in description_files] | ||
|
||
def _load_test_suite(self, test_suite: Dict[str, Any]) -> TestSuiteDescription: | ||
|
@@ -89,7 +89,7 @@ def _load_tests(source_file: Path) -> List[Type[AgentTest]]: | |
""" | ||
Takes a 'source_file', which must be a Python module, and returns a list of all the classes derived from AgentTest. | ||
""" | ||
spec = importlib.util.spec_from_file_location(f"tests_e2e.scenarios.{source_file.name}", str(source_file)) | ||
spec = importlib.util.spec_from_file_location(f"tests_e2e.tests.{source_file.name}", str(source_file)) | ||
module = importlib.util.module_from_spec(spec) | ||
spec.loader.exec_module(module) | ||
# return all the classes in the module that are subclasses of AgentTest but are not AgentTest itself. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,37 +15,40 @@ | |
# limitations under the License. | ||
# | ||
import contextlib | ||
import datetime | ||
import logging | ||
import re | ||
import traceback | ||
import uuid | ||
|
||
from assertpy import fail | ||
from enum import Enum | ||
from pathlib import Path | ||
from threading import current_thread, RLock | ||
from typing import Any, Dict, List | ||
|
||
# Disable those warnings, since 'lisa' is an external, non-standard, dependency | ||
# E0401: Unable to import 'lisa' (import-error) | ||
# E0401: Unable to import 'lisa.sut_orchestrator' (import-error) | ||
# E0401: Unable to import 'lisa.sut_orchestrator.azure.common' (import-error) | ||
# etc | ||
from lisa import ( # pylint: disable=E0401 | ||
CustomScriptBuilder, | ||
Logger, | ||
Node, | ||
notifier, | ||
TestCaseMetadata, | ||
TestSuite, | ||
TestSuiteMetadata, | ||
TestCaseMetadata, | ||
) | ||
from lisa.messages import TestStatus, TestResultMessage # pylint: disable=E0401 | ||
from lisa.sut_orchestrator import AZURE # pylint: disable=E0401 | ||
from lisa.sut_orchestrator.azure.common import get_node_context, AzureNodeSchema # pylint: disable=E0401 | ||
|
||
import makepkg | ||
from azurelinuxagent.common.version import AGENT_VERSION | ||
from tests_e2e.orchestrator.lib.agent_test_loader import AgentTestLoader, TestSuiteDescription | ||
from tests_e2e.scenarios.lib.agent_test_context import AgentTestContext | ||
from tests_e2e.scenarios.lib.identifiers import VmIdentifier | ||
from tests_e2e.scenarios.lib.logging import log as agent_test_logger # Logger used by the tests | ||
from tests_e2e.scenarios.lib.logging import set_current_thread_log | ||
from tests_e2e.tests.lib.agent_test_context import AgentTestContext | ||
from tests_e2e.tests.lib.identifiers import VmIdentifier | ||
from tests_e2e.tests.lib.logging import log as agent_test_logger # Logger used by the tests | ||
from tests_e2e.tests.lib.logging import set_current_thread_log | ||
|
||
|
||
def _initialize_lisa_logger(): | ||
|
@@ -97,8 +100,8 @@ class CollectLogs(Enum): | |
@TestSuiteMetadata(area="waagent", category="", description="") | ||
class AgentTestSuite(TestSuite): | ||
""" | ||
Base class for Agent test suites. It provides facilities for setup, execution of tests and reporting results. Derived | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot to update this comment in my previous PR |
||
classes use the execute() method to run the tests in their corresponding suites. | ||
Manages the setup of test VMs and execution of Agent test suites. This class acts as the interface with the LISA framework, which | ||
will invoke the execute() method when a runbook is executed. | ||
""" | ||
|
||
class _Context(AgentTestContext): | ||
|
@@ -294,7 +297,7 @@ def execute(self, node: Node, variables: Dict[str, Any], log: Logger) -> None: | |
""" | ||
self._set_context(node, variables, log) | ||
|
||
failed: List[str] = [] # List of failed tests (names only) | ||
test_suite_success = True | ||
|
||
with _set_thread_name(self.context.image_name): # The thread name is added to self._log | ||
try: | ||
|
@@ -308,11 +311,11 @@ def execute(self, node: Node, variables: Dict[str, Any], log: Logger) -> None: | |
test_suites: List[TestSuiteDescription] = AgentTestLoader(self.context.test_source_directory).load(self.context.test_suites) | ||
|
||
for suite in test_suites: | ||
failed.extend(self._execute_test_suite(suite)) | ||
test_suite_success = self._execute_test_suite(suite) and test_suite_success | ||
|
||
finally: | ||
collect = self.context.collect_logs | ||
if collect == CollectLogs.Always or collect == CollectLogs.Failed and len(failed) > 0: | ||
if collect == CollectLogs.Always or collect == CollectLogs.Failed and not test_suite_success: | ||
self._collect_node_logs() | ||
|
||
except: # pylint: disable=bare-except | ||
|
@@ -326,57 +329,84 @@ def execute(self, node: Node, variables: Dict[str, Any], log: Logger) -> None: | |
finally: | ||
self._clean_up() | ||
|
||
# Fail the entire test suite if any test failed; this exception is handled by LISA | ||
if len(failed) > 0: | ||
fail(f"{[self.context.image_name]} One or more tests failed: {failed}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to report this failure to LISA anymore. Now we report Pass/Fail for each individual test suite |
||
|
||
def _execute_test_suite(self, suite: TestSuiteDescription) -> List[str]: | ||
def _execute_test_suite(self, suite: TestSuiteDescription) -> bool: | ||
""" | ||
Executes the given test suite and returns True if all the tests in the suite succeeded. | ||
""" | ||
suite_name = suite.name | ||
suite_full_name = f"{suite_name}-{self.context.image_name}" | ||
|
||
with _set_thread_name(suite_full_name): # The thread name is added to self._log | ||
with set_current_thread_log(Path.home()/'logs'/f"{suite_full_name}.log"): | ||
agent_test_logger.info("") | ||
agent_test_logger.info("**************************************** %s ****************************************", suite_name) | ||
agent_test_logger.info("") | ||
|
||
failed: List[str] = [] | ||
summary: List[str] = [] | ||
|
||
for test in suite.tests: | ||
test_name = test.__name__ | ||
test_full_name = f"{suite_name}-{test_name}" | ||
|
||
agent_test_logger.info("******** Executing %s", test_name) | ||
self._log.info("******** Executing %s", test_full_name) | ||
|
||
try: | ||
|
||
test(self.context).run() | ||
|
||
summary.append(f"[Passed] {test_name}") | ||
agent_test_logger.info("******** [Passed] %s", test_name) | ||
self._log.info("******** [Passed] %s", test_full_name) | ||
except AssertionError as e: | ||
summary.append(f"[Failed] {test_name}") | ||
failed.append(test_full_name) | ||
agent_test_logger.error("******** [Failed] %s: %s", test_name, e) | ||
self._log.error("******** [Failed] %s", test_full_name) | ||
except: # pylint: disable=bare-except | ||
summary.append(f"[Error] {test_name}") | ||
failed.append(test_full_name) | ||
agent_test_logger.exception("UNHANDLED EXCEPTION IN %s", test_name) | ||
self._log.exception("UNHANDLED EXCEPTION IN %s", test_full_name) | ||
start_time: datetime.datetime = datetime.datetime.now() | ||
|
||
message: TestResultMessage = TestResultMessage() | ||
message.type = "AgentTestResultMessage" | ||
message.id_ = str(uuid.uuid4()) | ||
message.status = TestStatus.RUNNING | ||
message.suite_full_name = suite_name | ||
message.suite_name = message.suite_full_name | ||
message.full_name = f"{suite_name}-{self.context.image_name}" | ||
message.name = message.full_name | ||
message.elapsed = 0 | ||
notifier.notify(message) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We send those messages to our junit notifier. This first message indicates that a new test suite is running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sending the message at this point doesn't help us like we don't know live status. The pipeline only sends the report after test execution and at end of test execution we report with new status and that status override this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is part of the protocol needed by the implementation of the junit reporter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (this is required by LISA) |
||
|
||
try: | ||
agent_test_logger.info("") | ||
agent_test_logger.info("**************************************** %s ****************************************", suite_name) | ||
agent_test_logger.info("") | ||
|
||
agent_test_logger.info("********* [Test Results]") | ||
agent_test_logger.info("") | ||
for r in summary: | ||
agent_test_logger.info("\t%s", r) | ||
agent_test_logger.info("") | ||
failed: List[str] = [] | ||
summary: List[str] = [] | ||
|
||
for test in suite.tests: | ||
test_name = test.__name__ | ||
test_full_name = f"{suite_name}-{test_name}" | ||
|
||
agent_test_logger.info("******** Executing %s", test_name) | ||
self._log.info("******** Executing %s", test_full_name) | ||
|
||
try: | ||
|
||
test(self.context).run() | ||
|
||
summary.append(f"[Passed] {test_name}") | ||
agent_test_logger.info("******** [Passed] %s", test_name) | ||
self._log.info("******** [Passed] %s", test_full_name) | ||
except AssertionError as e: | ||
summary.append(f"[Failed] {test_name}") | ||
failed.append(test_name) | ||
agent_test_logger.error("******** [Failed] %s: %s", test_name, e) | ||
self._log.error("******** [Failed] %s", test_full_name) | ||
except: # pylint: disable=bare-except | ||
summary.append(f"[Error] {test_name}") | ||
failed.append(test_name) | ||
agent_test_logger.exception("UNHANDLED EXCEPTION IN %s", test_name) | ||
self._log.exception("UNHANDLED EXCEPTION IN %s", test_full_name) | ||
|
||
agent_test_logger.info("") | ||
|
||
agent_test_logger.info("********* [Test Results]") | ||
agent_test_logger.info("") | ||
for r in summary: | ||
agent_test_logger.info("\t%s", r) | ||
agent_test_logger.info("") | ||
|
||
if len(failed) == 0: | ||
message.status = TestStatus.PASSED | ||
else: | ||
message.status = TestStatus.FAILED | ||
message.message = f"Tests failed: {failed}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This just logs the failed test_names but if we could add some debug info or error message would be helpful. This would avoid downloads the log file especially if it's too many test failures and most of them known issues. If we know if it's known issue upfront, we could save time and makes it easy for people for monitors this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. Failures should not be frequent, but I'll consider that for a different iteration of the code. |
||
|
||
except: # pylint: disable=bare-except | ||
message.status = TestStatus.FAILED | ||
message.message = "Unhandled exception while executing test suite." | ||
message.stacktrace = traceback.format_exc() | ||
finally: | ||
message.elapsed = (datetime.datetime.now() - start_time).total_seconds() | ||
notifier.notify(message) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This second message indicates that the suite completed (either PASSED or FAILED) |
||
|
||
return failed | ||
return len(failed) == 0 | ||
|
||
def execute_script_on_node(self, script_path: Path, parameters: str = "", sudo: bool = False) -> int: | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
{ | ||
"name": "FailingTests", | ||
"name": "Fail", | ||
"tests": ["fail_test.py", "error_test.py"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
{ | ||
"name": "PassingTest", | ||
"name": "Pass", | ||
"tests": ["pass_test.py"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since now we are generating our own messages, I am restricting this to TestResultMessage instead of the base class, which also would include subtests (I may try subtests on subsequent PRs, but I need to see first if/how Azure Pipelines handles those)