Skip to content
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

issue: 4197123: Add Pytest to the workflow of the LogAnalyzer + issue: 4197133: Add Pytest #287

Merged
merged 14 commits into from
Dec 12, 2024

Conversation

Miryam-Schwartz
Copy link
Contributor

@Miryam-Schwartz Miryam-Schwartz commented Dec 8, 2024

What

Add pytest to log analyzer workflow and add some basics tests for this plugin.

Why ?

In order to test the plugin.

How ?

Pytest session was added to ufm_log_analyzer_ci_workflow.yml
unit_tesst directory was added with two simple unit tests.

Testing ?

Ensure tests passed.

Special triggers

Use the following phrases as comments to trigger different runs

  • bot:retest rerun Jenkins CI (to rerun GitHub CI, use "Checks" tab on PR page and rerun all jobs)
  • bot:upgrade run additional update tests

@Miryam-Schwartz Miryam-Schwartz changed the title test issue: 4197123: Add Pytest to the workflow of the LogAnalyzer + issue: 4197133: Add Pytest Dec 8, 2024
Copy link
Contributor

@drorlevy drorlevy left a comment

Choose a reason for hiding this comment

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

i am missing an example dump file / test inputs to the analyzer to analyze and verifying their response

@Miryam-Schwartz
Copy link
Contributor Author

@drorlevy , these are only unit tests and do not test the entire flow.

Copy link
Contributor

@drorlevy drorlevy left a comment

Choose a reason for hiding this comment

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

next tasks should include tests for each analyzer

Comment on lines 82 to 85
1. Make sure you have `pytest` installed. If not, you can install it using `pip`:

```bash
pip install pytest
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be clear that you use the venv created for the tool, right? otherwise tests might fail due to missing dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

return TestIBDIAGNETLogAnalyzer(fabric_size_data)

def test_get_fabric_size(analyzer, fabric_size_data):
# Call the method and check the result
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see the benefit in having this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@boazhaim boazhaim merged commit 38fb20b into main Dec 12, 2024
4 checks passed
@boazhaim boazhaim deleted the pytest branch December 12, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants