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

New test harness #1755

Merged
merged 1 commit into from
Oct 23, 2023
Merged

New test harness #1755

merged 1 commit into from
Oct 23, 2023

Conversation

aarnq
Copy link
Contributor

@aarnq aarnq commented Sep 5, 2023

What kind of PR is this?

Required: Mark one of the following that is applicable:

  • kind/feature
  • kind/improvement
  • kind/deprecation
  • kind/documentation
  • kind/clean-up
  • kind/bug

Optional: Mark one or more of the following that are applicable:

Important

Critical security fixes should be marked with kind/security
Breaking changes should be marked kind/admin-change or kind/dev-change depending on type

  • kind/admin-change
  • kind/dev-change
  • kind/security
  • kind/adr

What does this PR do / why do we need this PR?

This paves the way for more structured unit, integration, and end-to-end testing by using bats.

Additional information to reviewers

This has a long way to go before it can be used in the pipeline, and I want to integrate Cypress first so I know what requirements we have. I also aim to implement helpers for the current main test script and see if there is any way to generate tests from a matrix.

I've already did so for testing deployments and statefulsets, though we might want to rework it into a fetch-once-check-multiple solution as the old one did to improve the speed. Though with bats we can run things in parallel.

Cypress support is done! It is integrated with generators into the bats suite, but can be run manually as well. A support file is setup so we can add common functions if we need to, and a few config ones have been implemented.

There is also now a sort of generator for testing matrices, or at least the building blocks for doing so.

Screenshots

Checklist

  • Proper commit message prefix on all commits
  • Change checks:
    • The change is transparent
    • The change is disruptive
    • The change requires no migration steps
    • The change requires migration steps
  • Metrics checks:
    • The metrics are still exposed and present in Grafana after the change
    • The metrics names didn't change (Grafana dashboards and Prometheus alerts are not affected)
    • The metrics names did change (Grafana dashboards and Prometheus alerts were fixed)
  • Logs checks:
    • The logs do not show any errors after the change
  • Network Policy checks:
    • Any changed pod is covered by Network Policies
    • The change does not cause any dropped packages in the NetworkPolicy Dashboard
  • Pod Security Policy checks:
    • Any changed pod is covered by Pod Security Admission
    • Any changed pod is covered by Gatekeeper Pod Security Policies
    • The change does not cause any pods to be blocked by Pod Security Admission or Policies
  • Falco checks:
    • The change does not cause any alerts to be generated by Falco

@aarnq aarnq added the kind/feature New feature or request label Sep 5, 2023
@aarnq aarnq self-assigned this Sep 5, 2023
Copy link
Contributor

@robinAwallace robinAwallace left a comment

Choose a reason for hiding this comment

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

Looks very nice 👍

Is there any plan to check if resources have the correct values?
So that thanos have the expected variables and secrets/configmaps contains the expected values?

@aarnq
Copy link
Contributor Author

aarnq commented Sep 6, 2023

@robinAwallace that is not in the scope of this PR but in the future for sure.
I am working on methods of generating tests, and with that in place I think we could easily do loads of such tests. Because then you could just configure target cluster, resource, config key and config values and let it do the work.

@robinAwallace
Copy link
Contributor

Perfect, understandably that it is not part of this PR 👍

@aarnq aarnq force-pushed the aarnq/new-test-harness branch from 984ca61 to fc8a425 Compare September 12, 2023 11:13
@aarnq aarnq marked this pull request as ready for review September 12, 2023 12:04
@aarnq aarnq requested review from simonklb and Xartos September 13, 2023 09:49
tests/Dockerfile Outdated Show resolved Hide resolved
@aarnq aarnq force-pushed the aarnq/new-test-harness branch 7 times, most recently from f18a008 to 3d0e896 Compare September 15, 2023 12:34
@aarnq aarnq force-pushed the aarnq/new-test-harness branch from 3d0e896 to 9c3c903 Compare October 4, 2023 14:50
Copy link
Contributor

@Xartos Xartos left a comment

Choose a reason for hiding this comment

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

I think I would like to request a "meeting PR review" on this one. I don't think I'll be able to understand everything just by looking. Anyone else care to join? @robinAwallace @crssnd @simonklb

tests/Dockerfile Show resolved Hide resolved
Copy link
Contributor

@robinAwallace robinAwallace left a comment

Choose a reason for hiding this comment

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

As said on the retro, It looked very nice during the public PR and as such I will approve it. Good job André 👍

tests/README.md Outdated Show resolved Hide resolved
tests/README.md Show resolved Hide resolved
tests/README.md Show resolved Hide resolved
tests/common/cypress/support/lib.js Outdated Show resolved Hide resolved
tests/common/gen-bats.bash Outdated Show resolved Hide resolved
tests/common/gen-bats.bash Outdated Show resolved Hide resolved
Copy link
Contributor

@simonklb simonklb left a comment

Choose a reason for hiding this comment

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

Nice work!

I think we should consider adding the generate tests to the git repository though. I find it nice to track diffs when I update generated code. Interested to hear your take on it. (not blocking the merge btw)

@aarnq
Copy link
Contributor Author

aarnq commented Oct 23, 2023

Nice work!

I think we should consider adding the generate tests to the git repository though. I find it nice to track diffs when I update generated code. Interested to hear your take on it. (not blocking the merge btw)

Maybe, I don't think I want to add it right now to keep the structure simple.
But if we start to implement more features into the generators (and add the output change/tracking tests) then we will definitely go in that direction.

@aarnq aarnq force-pushed the aarnq/new-test-harness branch from c52de8e to 873e32f Compare October 23, 2023 07:52
@aarnq aarnq merged commit 873e32f into main Oct 23, 2023
15 checks passed
@aarnq aarnq deleted the aarnq/new-test-harness branch October 23, 2023 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants