Skip to content

restructure unit tests #12221

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

Merged
merged 1 commit into from
Feb 3, 2025
Merged

restructure unit tests #12221

merged 1 commit into from
Feb 3, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Feb 3, 2025

Motivation

In order to be easier to use test selection against our unit tests, and to fit a common format with our integration tests, this PR refactor our unit tests structure to both fit the rules, and be clearer. The AWS services unit tests were mixed up with common utils tests, docker client tests and such.

This way, we don't need to worry about file naming, and for new tests being added, it will be clearer which structure to follow in order to properly trigger test selection.

We're going from tests/unit/test_<service>.py to tests/unit/services/<service>/test_<whatever>.py

The CODEOWNERS have been manually updated, but our current generator is capable of handling the new format already and was already handling it, so it won't trigger issue.

Changes

  • restructure the unit tests to follow the same format as our integration tests
  • update the CODEOWNERS file manually to avoid pipeline failure, I'll run the generation manually to verify I haven't missed anything

@bentsku bentsku added the semver: patch Non-breaking changes which can be included in patch releases label Feb 3, 2025
@bentsku bentsku self-assigned this Feb 3, 2025
@bentsku bentsku marked this pull request as ready for review February 3, 2025 13:42
Copy link
Contributor

@tiurin tiurin left a comment

Choose a reason for hiding this comment

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

Nice improvement for tests maintainability @bentsku! Especially that some of the tests have already been under services. It's great that now we follow the same convention both inside unit tests and between unit and integration ones! 👏

Copy link

github-actions bot commented Feb 3, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 55m 2s ⏱️ - 25m 32s
4 075 tests ±0  3 760 ✅ ±0  315 💤 ±0  0 ❌ ±0 
4 077 runs  ±0  3 760 ✅ ±0  317 💤 ±0  0 ❌ ±0 

Results for commit 96c7412. ± Comparison against base commit 84b2f5f.

@bentsku bentsku added this to the 4.2 milestone Feb 3, 2025
@bentsku bentsku merged commit ca9275b into master Feb 3, 2025
41 checks passed
@bentsku bentsku deleted the structure-unit-tests branch February 3, 2025 15:05
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Nice and clean change! It applies the same structure as we already know from the integration tests, and it also works with the current CODEOWNERS automation. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants