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

Integration test and static main improvements #263

Merged
merged 13 commits into from
Nov 18, 2021

Conversation

nadiamoe
Copy link
Contributor

This PR moves the test data from the cmd/kubernetes-static folder to an internal/testutil one, and adds some goodies on top. This new testutil package contains:

  • A server based on httptest.Server which exposes all the required metrics from services for a particular version
  • A helper to create a fake kubernetes client and add some data to it.

With this, a WIP test for the new ksm.Scraper has been added as well.

Base automatically changed from pgallina/KSMrefactor to develop November 15, 2021 10:27
@nadiamoe nadiamoe force-pushed the integration-test-improvements branch 4 times, most recently from d4a6495 to f0428da Compare November 15, 2021 17:54
@nadiamoe nadiamoe self-assigned this Nov 16, 2021
@nadiamoe nadiamoe force-pushed the integration-test-improvements branch 2 times, most recently from 4a16a3a to f0a600f Compare November 16, 2021 15:53
kubernetes-static main has been migrated to the new backbone as well

Signed-off-by: Roberto Santalla <[email protected]>
Asserter is a (hopefully) powerful tool to assert that all metrics defined in a SpecGroup are correctly captured.

Signed-off-by: Roberto Santalla <[email protected]>
Signed-off-by: Roberto Santalla <[email protected]>
@nadiamoe nadiamoe force-pushed the integration-test-improvements branch from a1fd85d to 9a20034 Compare November 16, 2021 15:57
// AllVersions returns a list of versions we have test data for.
func AllVersions() []Version {
return []Version{
Testdata116,
Copy link
Member

Choose a reason for hiding this comment

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

should we add a comment saying to which KSM version corresponds to each of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this is something I though about. My plan is to overwrite all the static files so all of them use the KSM we support, but in the future we might want to have weirder versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a comment explaining this, and in the future, when I write the script to generate these files, I will keep in midn to log the version used somewhere.

Copy link
Member

@paologallinaharbur paologallinaharbur left a comment

Choose a reason for hiding this comment

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

A nice improvement!
I would try to reduce as much as possible "hidden" behaviour to make obvious what we are testing and using which data so that in 6 months we are not confused :)

@nadiamoe nadiamoe force-pushed the integration-test-improvements branch from 40392a5 to d7fd3b6 Compare November 17, 2021 16:18
I've added comments explaining how this works, which will hopefully improve readability.
Copy link
Member

@paologallinaharbur paologallinaharbur left a comment

Choose a reason for hiding this comment

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

Thanks for addressing everything!

@nadiamoe nadiamoe merged commit 942314f into develop Nov 18, 2021
@nadiamoe nadiamoe deleted the integration-test-improvements branch November 18, 2021 12:44
@nadiamoe nadiamoe mentioned this pull request Nov 18, 2021
16 tasks
paologallinaharbur pushed a commit that referenced this pull request Jan 14, 2022
* test: new integration testing backbone
kubernetes-static main has been migrated to the new backbone as well

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: make Version object return mocks

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement the Asserter
Asserter is a (hopefully) powerful tool to assert that all metrics defined in a SpecGroup are correctly captured.

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement helper for building integrations

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: log missing metric namespace

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: make asserter fully thread-safe

Signed-off-by: Roberto Santalla <[email protected]>

* test/ksm: optimize asserter calls on test parallelization

Signed-off-by: Roberto Santalla <[email protected]>

* fixup! test/asserter: typos

Signed-off-by: Roberto Santalla <[email protected]>

* testutil: make LatestVersion rely on the order of AllVersions

* testutil: do not wrap fake.NewSimpleClientset, but rather provide helpers with objects\
fixup! add namespaces

* testutil/asserter: check if specGroups are empty and fail in that case

* testutil/asserter: make Excluding copy-safe

* test/ksm: share the asserter again
I've added comments explaining how this works, which will hopefully improve readability.
paologallinaharbur pushed a commit that referenced this pull request Jan 17, 2022
* test: new integration testing backbone
kubernetes-static main has been migrated to the new backbone as well

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: make Version object return mocks

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement the Asserter
Asserter is a (hopefully) powerful tool to assert that all metrics defined in a SpecGroup are correctly captured.

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement helper for building integrations

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: log missing metric namespace

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: make asserter fully thread-safe

Signed-off-by: Roberto Santalla <[email protected]>

* test/ksm: optimize asserter calls on test parallelization

Signed-off-by: Roberto Santalla <[email protected]>

* fixup! test/asserter: typos

Signed-off-by: Roberto Santalla <[email protected]>

* testutil: make LatestVersion rely on the order of AllVersions

* testutil: do not wrap fake.NewSimpleClientset, but rather provide helpers with objects\
fixup! add namespaces

* testutil/asserter: check if specGroups are empty and fail in that case

* testutil/asserter: make Excluding copy-safe

* test/ksm: share the asserter again
I've added comments explaining how this works, which will hopefully improve readability.
paologallinaharbur pushed a commit that referenced this pull request Jan 17, 2022
* test: new integration testing backbone
kubernetes-static main has been migrated to the new backbone as well

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: make Version object return mocks

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement the Asserter
Asserter is a (hopefully) powerful tool to assert that all metrics defined in a SpecGroup are correctly captured.

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement helper for building integrations

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: log missing metric namespace

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: make asserter fully thread-safe

Signed-off-by: Roberto Santalla <[email protected]>

* test/ksm: optimize asserter calls on test parallelization

Signed-off-by: Roberto Santalla <[email protected]>

* fixup! test/asserter: typos

Signed-off-by: Roberto Santalla <[email protected]>

* testutil: make LatestVersion rely on the order of AllVersions

* testutil: do not wrap fake.NewSimpleClientset, but rather provide helpers with objects\
fixup! add namespaces

* testutil/asserter: check if specGroups are empty and fail in that case

* testutil/asserter: make Excluding copy-safe

* test/ksm: share the asserter again
I've added comments explaining how this works, which will hopefully improve readability.
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.

2 participants