Implementation of framework test harness (KEP-0008)

[jbarrick-mesosphere]

What type of PR is this?

/kind feature

What this PR does / why we need it:

This implements the framework test harness specified in KEP-0008.
Which issue(s) this PR fixes:

Fixes #284.

Special notes for your reviewer:

This PR is rather large, but it is mostly self contained and spiking on a new component, so this is expected.

I've put together a demo of this working to help reviewers understand what is happening: https://asciinema.org/a/249814

This is also specified in KEP-0008: https://github.com/kudobuilder/kudo/blob/master/keps/0008-framework-testing.md

We'll add full user documentation in a second PR (and improve the CLI interface), but to try this out:

# run tests against existing cluster
TEST_DIR=../../pkg/test/test_data go test ./cmd/test/... -tags build_harness -v
# run tests with mock control plane
START_CONTROL_PLANE=true START_KUDO=true CRD_DIR=../../config/crds/ MANIFESTS_DIR=../../config/samples/test-framework TESTS_DIR=../../pkg/test/test_data go test ./cmd/test/... -tags build_harness -v

Also see the help message:

go test ./cmd/test/... -tags build_harness -args -help

Does this PR introduce a user-facing change?:

Initial implementation of framework test harness.

[jbarrick-mesosphere]

I don't think I'm causing these test failures:

--- FAIL: TestInstallCmd (0.00s)
    install_test.go:51: 3: Found unexpected error: could not install framework(s): creating github client: missing github user
    install_test.go:56: 3: Missed expected error: could not install framework(s): getting config failed: Error loading config file "/tmp": read /tmp: is a directory
FAIL
coverage: 16.0% of statements
FAIL	github.com/kudobuilder/kudo/pkg/kudoctl/cmd/install	0.017s
?   	github.com/kudobuilder/kudo/pkg/kudoctl/cmd/plan	[no test files]
ok  	github.com/kudobuilder/kudo/pkg/kudoctl/util/check	0.014s	coverage: 59.1% of statements
--- FAIL: TestNewGithubClient (2.84s)
    client_test.go:49: 10:
        expected: client test: GET https://api.github.com/user: 401 Bad credentials []
             got: client test: GET https://api.github.com/user: 403 Maximum number of login attempts exceeded. Please try again later. []

[fabianbaier]

I don't think I'm causing these test failures:

--- FAIL: TestInstallCmd (0.00s)
    install_test.go:51: 3: Found unexpected error: could not install framework(s): creating github client: missing github user
    install_test.go:56: 3: Missed expected error: could not install framework(s): getting config failed: Error loading config file "/tmp": read /tmp: is a directory
FAIL
coverage: 16.0% of statements
FAIL	github.com/kudobuilder/kudo/pkg/kudoctl/cmd/install	0.017s
?   	github.com/kudobuilder/kudo/pkg/kudoctl/cmd/plan	[no test files]
ok  	github.com/kudobuilder/kudo/pkg/kudoctl/util/check	0.014s	coverage: 59.1% of statements
--- FAIL: TestNewGithubClient (2.84s)
    client_test.go:49: 10:
        expected: client test: GET https://api.github.com/user: 401 Bad credentials []
             got: client test: GET https://api.github.com/user: 403 Maximum number of login attempts exceeded. Please try again later. []

You don't and we are getting rid of the Github Client and its test, so don't worry about it.

[orsenthil]

• One comment to explain the difference between teststage vs testcase, specifically why is it called teststage in the README itself.
• Couple of minor refactor comments - This can be addressed if it is agreed upon by KUDO maintainers.

Thank you! LGTM.

[orsenthil]

Do you need to explain the relationship between test stage and a test case?

Why is it called a Test stage instead of test case which is a more common term?

[oliviabarrick]

I decided to change it to stage specifically to differentiate it from the definition of case.

We have:

• Tests (we could rename these to test cases) which are the independent tests
• Test stages are parts of a single test. They get run sequentially and depend on the status of the previous one. The stages all form one test, so IMO calling them test cases is confusing.

[orsenthil]

FYI, In other worlds, the nouns are TestSuite, TestCase, and test.

The shared understanding is:
Test Suite is comprises of Test Cases which comprises of Tests.

Test stage is a new term. If we agree to adopt it, it is fine. (Test stage seems like a single test function).

A test case defines the fixture/class to run multiple tests.
http://junit.sourceforge.net/junit3.8.1/javadoc/junit/framework/TestCase.html

[oliviabarrick]

How about:

• Test Harness: the thing that runs a test suite.
• Test Suite: the collection of all test cases.
• Test Case: a single, self contained test - can be run in parallel to other test cases.
• Test Step (or TestStage, but I thought of TestStep last night and it seems maybe even more clear): a portion of a test case indicating a state to apply and expect, dependent on all previous test steps in the test case being successful.

[orsenthil]

This sounds much better to me. Does not need any further explanation as it goes along with what is commonly well understood.

"Stage" is a term also used the building software where there are dependencies and ordering between stages.

[fabianbaier]

A bit late to this conversation but when reading through it I want to also maybe add some idea or perspective. Imo it reminded me a bit on the hierarchy and naming established within Phases. So just something that maybe spark some good thoughts:

TestPhase for Test Suite if aligned with how we think about the name and what a Plan is here https://github.com/kudobuilder/kudo/blob/master/keps/0009-operator-toolkit.md#plans ?
TestStep for Test Case if aligned with how we think about the name and what a Step is here https://github.com/kudobuilder/kudo/blob/master/keps/0009-operator-toolkit.md#steps ?
TestTask for Test Step if aligned with how we think about the name and what a Step is here https://github.com/kudobuilder/kudo/blob/master/keps/0009-operator-toolkit.md#tasks ?

Maybe I am entirely wrong here and this won't work, again just a thought!

[oliviabarrick]

Note: we’ll also integrate this into kudoctl in a follow up PR.

[oliviabarrick]

This also does not yet fully implement the design as laid out in the KEP. In follow up PRs, we’ll add:

• Making sure that all resources delete cleanly
• Automatically verify object status if an assert is not provided for it.
• Test constraints.
• Implement the delete option for test steps.

[oliviabarrick]

An open question: right now, if you provide the flags to install CRDs or manifests prior to running the tests, it will bail out if any of the resources already exist. I have not decided the safest way to handle this case. We could:

• Ignore resources that already exist. This has the drawback that their frameworks or CRDs may be out of date when running tests.
• Update the resources that already exist. This has the drawback that we might unexpectedly replace production framework versions or CRDs if run in a production cluster.
• Bail out and let the user decide how to handle the situation (remove flags and re-run).

[alenkacz]

OK I re-read the KEP and did one initial pass in the PR. I did not really focus on the golang code yet, so I am not submitting a review right now. But in general this feels like very generic test framework for kubernetes. Shouldn't it at least live in a separate repo?

the only thing that relates it to kudo is pretty much this, right? https://github.com/kudobuilder/kudo/pull/285/files#diff-426fa14ee729b01f7853a36a1e8542aeR224

EDIT: Or do we plan to use this even for integration testing the base tech (kudo?). The KEP was not really clear on that

[oliviabarrick]

OK I re-read the KEP and did one initial pass in the PR. I did not really focus on the golang code yet, so I am not submitting a review right now. But in general this feels like very generic test framework for kubernetes. Shouldn't it at least live in a separate repo?

the only thing that relates it to kudo is pretty much this, right? https://github.com/kudobuilder/kudo/pull/285/files#diff-426fa14ee729b01f7853a36a1e8542aeR224

EDIT: Or do we plan to use this even for integration testing the base tech (kudo?). The KEP was not really clear on that

We can use this to test the controller as well as frameworks, so yeah it’s already intended to be used in two repositories. I could definitely see it being useful for other projects (in and outside of Mesosphere), so a separate repository definitely makes a lot of sense to me. I’m not sure what the other kudo contributors think about that, though, as right now kudo has total control over the design and I’m not sure how much moving it to a different repository would affect that.

[jbarrick-mesosphere]

Rebased.

[runyontr]

LGTM, Lets get this in and start using it!