-
Notifications
You must be signed in to change notification settings - Fork 372
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
Bug fixes for test pipeline #2764
Changes from 1 commit
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 |
---|---|---|
|
@@ -104,10 +104,12 @@ def _validate(self): | |
if suite.images not in self.images: | ||
raise Exception(f"Invalid image reference in test suite {suite.name}: Can't find {suite.images} in images.yml") | ||
|
||
# If the suite specifies a location, validate that the images are available in that location | ||
# If the suite specifies a location, validate that the images it uses are available in that location | ||
if suite.location != '': | ||
if not any(suite.location in i.locations for i in self.images[suite.images]): | ||
raise Exception(f"Test suite {suite.name} must be executed in {suite.location}, but no images in {suite.images} are available in that location") | ||
for image in self.images[suite.images]: | ||
if len(image.locations) > 0: | ||
if suite.location not in image.locations: | ||
raise Exception(f"Test suite {suite.name} must be executed in {suite.location}, but <{image.urn}> is not available in that location") | ||
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. Fixed error message to use the URN for the image blocking the test |
||
|
||
@staticmethod | ||
def _load_test_suites(test_suites: str) -> List[TestSuiteInfo]: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,6 @@ | |
import traceback | ||
import uuid | ||
|
||
from enum import Enum | ||
from pathlib import Path | ||
from threading import current_thread, RLock | ||
from typing import Any, Dict, List | ||
|
@@ -91,7 +90,7 @@ def _set_thread_name(name: str): | |
# | ||
# Possible values for the collect_logs parameter | ||
# | ||
class CollectLogs(Enum): | ||
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. comparing a string to one of the enum items was failing; changed this to be a class with string data members instead of an Enum |
||
class CollectLogs(object): | ||
Always = 'always' # Always collect logs | ||
Failed = 'failed' # Collect logs only on test failures | ||
No = 'no' # Never collect logs | ||
|
@@ -341,14 +340,15 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool: | |
suite_full_name = f"{suite_name}-{self.context.image_name}" | ||
suite_start_time: datetime.datetime = datetime.datetime.now() | ||
|
||
success: bool = True # True if all the tests succeed | ||
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 method is supposed to return True on success; it was returning True on failure |
||
|
||
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"): | ||
try: | ||
agent_test_logger.info("") | ||
agent_test_logger.info("**************************************** %s ****************************************", suite_name) | ||
agent_test_logger.info("") | ||
|
||
failed: bool = False # True if any test fails | ||
summary: List[str] = [] | ||
|
||
for test in suite.tests: | ||
|
@@ -372,7 +372,7 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool: | |
TestStatus.PASSED, | ||
test_start_time) | ||
except AssertionError as e: | ||
failed = True | ||
success = False | ||
summary.append(f"[Failed] {test_name}") | ||
agent_test_logger.error("******** [Failed] %s: %s", test_name, e) | ||
self._log.error("******** [Failed] %s", test_full_name) | ||
|
@@ -383,7 +383,7 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool: | |
test_start_time, | ||
message=str(e)) | ||
except: # pylint: disable=bare-except | ||
failed = True | ||
success = False | ||
summary.append(f"[Error] {test_name}") | ||
agent_test_logger.exception("UNHANDLED EXCEPTION IN %s", test_name) | ||
self._log.exception("UNHANDLED EXCEPTION IN %s", test_full_name) | ||
|
@@ -404,7 +404,7 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool: | |
agent_test_logger.info("") | ||
|
||
except: # pylint: disable=bare-except | ||
failed = True | ||
success = False | ||
self._report_test_result( | ||
suite_full_name, | ||
suite_name, | ||
|
@@ -413,7 +413,7 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool: | |
message=f"Unhandled exception while executing test suite {suite_name}.", | ||
add_exception_stack_trace=True) | ||
|
||
return failed | ||
return success | ||
|
||
@staticmethod | ||
def _report_test_result( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,11 @@ extension: | |
- "../lib" | ||
|
||
variable: | ||
# | ||
# These variables identify the existing VM, and the user for SSH connections | ||
# | ||
- name: subscription_id | ||
value: "" | ||
|
||
- name: resource_group_name | ||
value: "" | ||
- name: vm_name | ||
|
@@ -44,7 +46,61 @@ variable: | |
value: "" | ||
is_secret: true | ||
|
||
# Set these to use an SSH proxy | ||
# | ||
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 add the combinator to this runbook in my previous PR |
||
# These variables define parameters for the AgentTestSuite; see the test wiki for details. | ||
# | ||
# NOTE: c_test_suites, generated by the AgentTestSuitesCombinator, is also a parameter | ||
# for the AgentTestSuite | ||
# | ||
# Whether to collect logs from the test VM | ||
- name: collect_logs | ||
value: "failed" | ||
is_case_visible: true | ||
|
||
# Whether to skip setup of the test VM | ||
- name: skip_setup | ||
value: false | ||
is_case_visible: true | ||
|
||
# | ||
# These variables are parameters for the AgentTestSuitesCombinator | ||
# | ||
# The test suites to execute | ||
- name: test_suites | ||
value: "agent_bvt" | ||
- name: image | ||
value: "" | ||
- name: location | ||
value: "" | ||
- name: vm_size | ||
value: "" | ||
|
||
# | ||
# The values for these variables are generated by the AgentTestSuitesCombinator combinator. They are | ||
# prefixed with "c_" to distinguish them from the rest of the variables, whose value can be set from | ||
# the command line. | ||
# | ||
# c_marketplace_image, c_vm_size, c_location, and c_vhd are handled by LISA and define | ||
# the set of test VMs that need to be created, while c_test_suites is a parameter | ||
# for the AgentTestSuite and defines the test suites that must be executed on each | ||
# of those test VMs (the AgentTestSuite also uses c_vhd) | ||
# | ||
- name: c_marketplace_image | ||
value: "" | ||
- name: c_vm_size | ||
value: "" | ||
- name: c_location | ||
value: "" | ||
- name: c_vhd | ||
value: "" | ||
is_case_visible: true | ||
- name: c_test_suites | ||
value: [] | ||
is_case_visible: true | ||
|
||
# | ||
# Set these variables to use an SSH proxy when executing the runbook | ||
# | ||
- name: proxy | ||
value: False | ||
- name: proxy_host | ||
|
@@ -55,19 +111,6 @@ variable: | |
value: "" | ||
is_secret: true | ||
|
||
# These variables define parameters for the AgentTestSuite | ||
- name: test_suites | ||
value: "agent_bvt" | ||
is_case_visible: true | ||
|
||
- name: collect_logs | ||
value: "failed" | ||
is_case_visible: true | ||
|
||
- name: skip_setup | ||
value: true | ||
is_case_visible: true | ||
|
||
platform: | ||
- type: azure | ||
admin_username: $(user) | ||
|
@@ -81,6 +124,13 @@ platform: | |
location: $(location) | ||
name: $(vm_name) | ||
|
||
combinator: | ||
type: agent_test_suites | ||
test_suites: $(test_suites) | ||
image: $(image) | ||
location: $(location) | ||
vm_size: $(vm_size) | ||
|
||
notifier: | ||
- type: env_stats | ||
- type: agent.junit | ||
|
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.
The check below needs to be done only when the image specifies a location