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

[DPE-2651] Juju3 pipelines / Testing secrets #333

Merged
merged 12 commits into from Oct 16, 2023
Merged

Conversation

juditnovak
Copy link
Contributor

@juditnovak juditnovak commented Oct 2, 2023

Issue

JujuVersion.has_secrets have to be correctly patched so that secrets-related code would be invoked

Solution

Pytest fixtures ensuring the usage to JujuVersion.has_secrets (instead of its default behavior returning False all way through)

NOTE: The 3 pipeline failures are unrelated to secrets. They will be addressed in upcoming PRs as below.
This PR will be merged AFTER those.

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #333 (167f2a0) into main (1f72f1b) will increase coverage by 0.96%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   64.50%   65.46%   +0.96%     
==========================================
  Files          17       17              
  Lines        3113     3113              
  Branches      413      413              
==========================================
+ Hits         2008     2038      +30     
+ Misses        979      946      -33     
- Partials      126      129       +3     
Files Coverage Δ
src/relations/mysql_provider.py 37.80% <50.00%> (ø)

... and 1 file with indirect coverage changes

@juditnovak juditnovak force-pushed the DPE-2651_testing_secrets branch 3 times, most recently from 466af7c to fa03650 Compare October 4, 2023 07:04
tox.ini Outdated Show resolved Hide resolved
@juditnovak juditnovak marked this pull request as ready for review October 4, 2023 13:29
shayancanonical
shayancanonical previously approved these changes Oct 5, 2023
Copy link
Contributor

@shayancanonical shayancanonical left a comment

Choose a reason for hiding this comment

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

Looks amazing!

tests/conftest.py Show resolved Hide resolved
shayancanonical
shayancanonical previously approved these changes Oct 5, 2023
Copy link
Contributor

@carlcsaposs-canonical carlcsaposs-canonical left a comment

Choose a reason for hiding this comment

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

If we're planning to support multiple juju versions long-term, I think we might want to rework the fixture so that it's only for integration tests and unit tests for all juju versions run at the same time

But if we only need to support juju 2+3 for a short time this looks good

Also I think the skip vs deselect for tests might be a bit confusing with the UX that we're using to group integration tests, but since we're not heavily using groups we probably won't encounter that often for now

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
tests/conftest.py Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
shayancanonical
shayancanonical previously approved these changes Oct 6, 2023
@juditnovak juditnovak force-pushed the DPE-2651_testing_secrets branch 4 times, most recently from 9c5434d to 6aaaeb0 Compare October 6, 2023 12:36
@juditnovak juditnovak changed the title [DPE-2651] Testing secrets [DPE-2651] Juju3 pipelines / Testing secrets Oct 13, 2023
paulomach
paulomach previously approved these changes Oct 13, 2023
Copy link
Contributor

@paulomach paulomach left a comment

Choose a reason for hiding this comment

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

excellent, thank you

@delgod delgod merged commit 005a573 into main Oct 16, 2023
27 of 29 checks passed
@delgod delgod deleted the DPE-2651_testing_secrets branch October 16, 2023 13:05
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.

None yet

6 participants