-
Notifications
You must be signed in to change notification settings - Fork 22
CIV: Report Portal Integration with AMI Validation #284
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| [pytest] | ||
| render_collapsed = True | ||
| markers = | ||
| pub: Tests that have to be run on images that are already published into cloud providers marketplaces. | ||
| wait: Number of seconds the test should wait before it starts. | ||
| run_on: List of distro-version (or only distro) combinations where the test should be executed. | ||
| exclude_on: List of distro-version (or only distro) combinations where the test should NOT be executed. | ||
| jira_skip: List of Jira ticker ids. If any of these tickets are not closed, the test will be skipped. | ||
| markers = | ||
| pub: Tests that have to be run on images that are already published into cloud providers marketplaces. | ||
| wait: Number of seconds the test should wait before it starts. | ||
| run_on: List of distro-version (or only distro) combinations where the test should be executed. | ||
| exclude_on: List of distro-version (or only distro) combinations where the test should NOT be executed. | ||
| jira_skip: List of Jira ticker ids. If any of these tickets are not closed, the test will be skipped. | ||
| rp_verify_ssl = False | ||
|
Collaborator
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. Let's add this options programmatically with the new class we created (PytestConfig). |
||
| rp_is_skipped_an_issue = False | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,3 +16,4 @@ paramiko==3.1.0 | |
| awscli==1.27.119 | ||
| requests==2.28.1 | ||
| packaging==23.2 | ||
| pytest-reportportal==5.3.0 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,8 @@ class TestCloudImageValidator: | |
| 'parallel': True, | ||
| 'debug': True, | ||
| 'stop_cleanup': False, | ||
| 'config_file': '/tmp/test_config_file.yml', | ||
| 'report_portal': False, | ||
| 'config_file': '/tmp/test_config_file.yml' | ||
| } | ||
| test_instances = { | ||
| 'instance-1': {'public_dns': 'value_1', 'username': 'value_2'}, | ||
|
|
@@ -134,7 +135,8 @@ def test_run_tests_in_all_instances(self, mocker, validator): | |
| validator.run_tests_in_all_instances(self.test_instances) | ||
|
|
||
| mock_run_tests.assert_called_once_with( | ||
| validator.config["output_file"], self.test_config["test_filter"], self.test_config["include_markers"]) | ||
| validator.config["output_file"], self.test_config["test_filter"], self.test_config["include_markers"], | ||
|
Collaborator
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. Maybe we can format the args like this for easier readability: |
||
| self.test_config["report_portal"]) | ||
|
|
||
| def test_destroy_infrastructure(self, mocker, validator): | ||
| mock_destroy_infra = mocker.patch.object( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import json | ||
| import logging | ||
| import os | ||
| import re | ||
| import time | ||
|
|
@@ -8,12 +9,21 @@ | |
| from packaging import version | ||
| from py.xml import html | ||
| from pytest_html import extras | ||
| from reportportal_client import RPLogger | ||
| from requests.adapters import HTTPAdapter | ||
| from requests.packages.urllib3.util.retry import Retry | ||
|
|
||
| from lib import test_lib | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def rp_logger(): | ||
|
Collaborator
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. Is there a way of defining this fixture only if the arg
Collaborator
Author
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. mmm... I'm not sure, need to investigate this - maybe a marker for this use?
Collaborator
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. If this fixture does not affect current logic/report (in the case we don't use
Collaborator
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. What about the custom attributes we plan to add for each test case? Will that be done in a separate pull request?
Collaborator
Author
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. According to the examples provided here https://github.com/reportportal/agent-python-pytest#readme I think that we don't need to make a second rest API call for this, we need to create markers on top of TestsAWS class and these markers will be attached as attributes to Report Portal items.
Collaborator
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. Yes, it makes sense to create as separate ticket for this and another PR.
Collaborator
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 would add a if guard here, checking if CIVConfig contains |
||
| logger = logging.getLogger(__name__) | ||
| logger.setLevel(logging.DEBUG) | ||
| logging.setLoggerClass(RPLogger) | ||
| return logger | ||
|
|
||
|
|
||
| def __get_host_info(host): | ||
| host_info = {} | ||
| host_info['distro'] = host.system_info.distribution | ||
|
|
||
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.
Let's make this part optional, only in the case
--rpoption is passed as argument.