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

WIP: [POC] Refactor: port unittest to pytest #3361

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Mikaayenson
Copy link
Contributor

@Mikaayenson Mikaayenson commented Jan 3, 2024

Issues

Related to https://github.com/elastic/ia-trade-team/issues/238

Sets up some foundational changes that we plan to make to support Detections as Code. We want to be able to use more useful features of pytest and the current setup of intermingling python unittest while we actually already use pytest complicated our ability to do this. Migrating to a single testing capability removes some of this confusion and sets us up for the next enhancements to come.

Summary

  • Refactors the test suite from unittest to pytest. This is this first of several PRs to come. Initially I wanted to add in the pytest mark labels, but we'll save that for a subsequent PR to limit the scope of changes.
  • Migrates base.py to conftests.py (pytest magic file)
  • Also found a bug in one of the unittest TestPackages::test_rule_versioning where we weren't actually iterating over the rules.

Testing

Unit tests should all still pass as before.

@Mikaayenson Mikaayenson added bug Something isn't working enhancement New feature or request test-suite unit and other testing components python Internal python for the repository labels Jan 3, 2024
@Mikaayenson Mikaayenson self-assigned this Jan 3, 2024
Copy link
Contributor

@terrancedejesus terrancedejesus left a comment

Choose a reason for hiding this comment

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

Port seems fine. Checked out branch, ran tests collectively and then individually and they worked.

@Mikaayenson Mikaayenson changed the title [FR] refactor unittest to pytest [FR] Refactor: port unittest to pytest Jan 5, 2024
Copy link
Contributor

@eric-forte-elastic eric-forte-elastic left a comment

Choose a reason for hiding this comment

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

Ports look good! Tests work locally.

@Mikaayenson
Copy link
Contributor Author

FWIW, I manually compared the time it took to run the unit tests with other PRs and the time to complete has not changed.

@Mikaayenson Mikaayenson changed the title [FR] Refactor: port unittest to pytest WIP: [FR] Refactor: port unittest to pytest Jan 22, 2024
@Mikaayenson Mikaayenson changed the title WIP: [FR] Refactor: port unittest to pytest WIP: [POC] Refactor: port unittest to pytest Jan 22, 2024
@Mikaayenson Mikaayenson marked this pull request as draft March 14, 2024 22:45
@Mikaayenson
Copy link
Contributor Author

Converted to draft so it doesnt accidentally get merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog backport: auto bug Something isn't working detections-as-code enhancement New feature or request python Internal python for the repository test-suite unit and other testing components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants