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

refactor(pytest): make test more robust and easier to debug #598

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

tiagolobocastro
Copy link
Contributor

@tiagolobocastro tiagolobocastro commented Jan 9, 2025

refactor(nix): make shell nix depend on other shells

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

ci(bdd): use larger runner for testing

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

test(bdd): make test more robust

Ensure install is properly completed
Ensure upgrade is properly completed
Compare against v-next and not v-next since there's no current way
of knowing which version is the current helm chart.
This may get addressed if we start building the images and the
chart as v-next from the outset.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

test: ensure hugepages allocated

When deploying a kind cluster, ensure we allocate the requested hugepages.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

refactor(pytest): make debugging easier

Change a lot of return Nones for exceptions which aids debugging by
failing at the point of failure
Reduce verbose logging when install/upgrades takes some time due to
cpu or network constraints
Remove extra logging
Allow using kubectl plugin from path

I think the tests are currently broken for release branches as its
asserting for develop.
Also we're modifying the chart itself, which is not ideal.
todo: improve this

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

@tiagolobocastro
Copy link
Contributor Author

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Jan 9, 2025
@bors-openebs-mayastor
Copy link
Contributor

try

Build failed:

Change a lot of return Nones for exceptions which aids debugging by
failing at the point of failure
Reduce verbose logging when install/upgrades takes some time due to
cpu or network constraints
Remove extra logging
Allow using kubectl plugin from path

I think the tests are currently broken for release branches as its
asserting for develop.
Also we're modifying the chart itself, which is not ideal.
todo: improve this

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro tiagolobocastro force-pushed the pytest branch 2 times, most recently from f82d8b0 to be41609 Compare January 10, 2025 13:36
When deploying a kind cluster, ensure we allocate the requested hugepages.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Ensure install is properly completed
Ensure upgrade is properly completed
Compare against v-next and not v-next since there's no current way
of knowing which version is the current helm chart.
This may get addressed if we start building the images and the
chart as v-next from the outset.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro tiagolobocastro force-pushed the pytest branch 5 times, most recently from d647b79 to cadbe0c Compare January 13, 2025 14:26
@tiagolobocastro tiagolobocastro changed the title refactor(pytest): make debugging easier refactor(pytest): make test more robust and easier to debug Jan 13, 2025
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro tiagolobocastro force-pushed the pytest branch 3 times, most recently from 4b6ec90 to 10d1a92 Compare January 14, 2025 11:52
Allows the bdd tests to run and setup the vnext chart on their own
We can still pre-deploy the kind cluster on CI for time tracking

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Copy link
Member

@niladrih niladrih left a comment

Choose a reason for hiding this comment

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

Lot of work!

.github/workflows/k8s-ci.yml Outdated Show resolved Hide resolved
tests/bdd/common/k8s_deployer.py Outdated Show resolved Hide resolved
@tiagolobocastro
Copy link
Contributor Author

Still missing is "forking" the chart, I'll leave that for another PR since requires modifying dependencies release script to allow for custom args in order to select non-custom location for the chart
bors merge

bors-openebs-mayastor bot pushed a commit that referenced this pull request Jan 15, 2025
598: refactor(pytest): make test more robust and easier to debug r=tiagolobocastro a=tiagolobocastro

    refactor(nix): make shell nix depend on other shells
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>



Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro
Copy link
Contributor Author

bors cancel

@bors-openebs-mayastor
Copy link
Contributor

Canceled.

@tiagolobocastro
Copy link
Contributor Author

bors merge

@bors-openebs-mayastor
Copy link
Contributor

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

bors-openebs-mayastor bot pushed a commit that referenced this pull request Jan 15, 2025
598: refactor(pytest): make test more robust and easier to debug r=tiagolobocastro a=tiagolobocastro

    refactor(nix): make shell nix depend on other shells
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>



Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>
@bors-openebs-mayastor
Copy link
Contributor

This PR was included in a batch that successfully built, but then failed to merge into develop. It will not be retried.

Additional information:

Response status code: 422
{"message":"Required status check \"helm-chart-test\" is expected.","documentation_url":"https://docs.github.com/articles/about-protected-branches","status":"422"}

When pushing tags (annotated?), github's event.base_ref may not be set.
When this happens, we can figure out the released branch ourselves, by
inspecting all branches containing the tag, and ensuring it's the tip
and it's the current HEAD.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro
Copy link
Contributor Author

Some weird github actions thing... I made only bors a requirement, since it pulls in the others anyway...
bors merge

@bors-openebs-mayastor
Copy link
Contributor

Build succeeded:

@bors-openebs-mayastor bors-openebs-mayastor bot merged commit c95e617 into develop Jan 16, 2025
5 checks passed
@bors-openebs-mayastor bors-openebs-mayastor bot deleted the pytest branch January 16, 2025 00:37
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