CIV: Report Portal Integration with AMI Validation#284
CIV: Report Portal Integration with AMI Validation#284sshmulev wants to merge 1 commit intoosbuild:mainfrom
Conversation
|
Please update the README.md explaining how the new feature works and how to use it (env vars, etc.). |
|
Don't forget to update the unit tests 👍 |
test_suite/conftest.py
Outdated
| return logger | ||
|
|
||
|
|
||
| def get_rp_config(): |
There was a problem hiding this comment.
Maybe we can move this function to config_lib.py. We may also introduce a new class in that file, which can be called PytestConfig. It can be useful in the future to actually add more config there directly, instead of having it hardcoded in pytest.ini.
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def rp_logger(): |
There was a problem hiding this comment.
Is there a way of defining this fixture only if the arg -rp (or --report-portal) is used?
There was a problem hiding this comment.
mmm... I'm not sure, need to investigate this - maybe a marker for this use?
There was a problem hiding this comment.
If this fixture does not affect current logic/report (in the case we don't use -rp option), then it's not a big deal and we can leave it enabled at a session level by default. I wonder if this has performance implications though.
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def rp_logger(): |
There was a problem hiding this comment.
What about the custom attributes we plan to add for each test case? Will that be done in a separate pull request?
I can imagine it will be a fixture defined like this: @pytest.fixture(autouse=True, scope='function').
There was a problem hiding this comment.
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.
I think I'll work on this in a different PR because it might take more time probabbly , what do you think?
There was a problem hiding this comment.
Yes, it makes sense to create as separate ticket for this and another PR.
Just one comment for future PR: Let's remember we need custom attributes for each test case separately, and they won't be fixed attributes, they will be dynamic, their values will vary during runtime (e.g. AMI ID, region, instance ID, public dns, etc.).
sshmulev
force-pushed
the
report_portal_agent
branch
14 times, most recently
from
ef020cc to
d7f0f17
Compare
|
Let's add the report portal pytest agent in requirements.txt |
sshmulev
force-pushed
the
report_portal_agent
branch
from
d7f0f17 to
83db364
Compare
| os.environ['PYTHONPATH'] = ':'.join( | ||
| [f'{os.path.dirname(__file__)}', os.environ['PYTHONPATH']]) | ||
|
|
||
| rp_config = PytestConfig() |
There was a problem hiding this comment.
Let's make this part optional, only in the case --rp option is passed as argument.
| 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 |
There was a problem hiding this comment.
Let's add this options programmatically with the new class we created (PytestConfig).
|
|
||
| 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"], |
There was a problem hiding this comment.
Maybe we can format the args like this for easier readability:
mock_run_tests.assert_called_once_with(validator.config["output_file"],
self.test_config["test_filter"],
self.test_config["include_markers"],
self.test_config["report_portal"])
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def rp_logger(): |
There was a problem hiding this comment.
I would add a if guard here, checking if CIVConfig contains report_portal.
If not, just return and don't continue with current logic.
2 participants
No description provided.