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

Test all api versions #1473

Closed
lavalamp opened this issue Sep 26, 2014 · 9 comments
Closed

Test all api versions #1473

lavalamp opened this issue Sep 26, 2014 · 9 comments
Assignees
Labels
area/reliability area/test area/test-infra priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@lavalamp
Copy link
Member

We need to make our tests parameterized for api version, e.g., via a command-line flag. Then we need to make hack/test-go.sh run our tests once for each version we support.

Ad-hoc testing of different versions is going to be error prone and repeat a bunch of logic unnecessarily.

Concrete steps:

a. Make an api/test library
b. It defines a command-line flag
c. It defines a Codec, etc (like the api/latest package) which is set based off of the command line flag
d. Tests everywhere are changed to import this library and use its Codec.
e. The testing script runs tests once for each version.

@lavalamp lavalamp added this to the v0.7 milestone Sep 27, 2014
@smarterclayton
Copy link
Contributor

What about tests that cover features specific to a version, or conversion between? Maybe move all of those to use latest.Version so we don't accidentally delete them for appearing old.

@lavalamp
Copy link
Member Author

Those tests just exit early if the passed in flag doesn't match their target version.

@bgrant0607 bgrant0607 modified the milestones: v0.8, v0.7 Oct 27, 2014
@bgrant0607 bgrant0607 added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Dec 3, 2014
@goltermann goltermann removed this from the v0.8 milestone Feb 6, 2015
@lavalamp lavalamp removed this from the v0.8 milestone Feb 6, 2015
@davidopp
Copy link
Member

Should we consider this required for 1.0? Sorry for the super-naive question, but how are we handling this today? Do we only test v1? Which tests actually depend on the API version?

@nikhiljindal
Copy link
Contributor

Yes. We were testing only v1beta1.
I recently submitted a PR to test both versions (v1beta1 and 3) in our integration tests: #5587
I am now working on updating other tests as well.
Tracking that as part of: #5475

@ixdy
Copy link
Member

ixdy commented Mar 20, 2015

As I commented in #5687, I'm a bit worried about testing all API versions in hack/test-go.sh by default. This will result in make test being N times slower, and I'm not sure that Travis will be able to keep up (as it is already having a bit of trouble) - at the very least we would probably need to disable coverage on most runs on Travis.

@nikhiljindal
Copy link
Contributor

Thanks for looking at that PR @ixdy.
Thats a valid concern.
For a short time, after we enable v1beta3 and before we deprecate v1beta1, we need to test both the API versions.
Looks like /hack/test-go takes about 2 minutes right now to run on travis. So we will be increasing its run time by 2 minutes by running it twice.
We can try it and see if travis behaves fine.
If not, we can pick a random version each time and run the tests for that (a bit like what our fuzzer tests do now). Maybe we can do that just on travis and run the tests for both the versions, when it is run locally?
We can even start with that first and see how that works instead.

What do you think?

@smarterclayton
Copy link
Contributor

Testing all version in travis makes sense (but not by default). If it's an ENV var switch it should be clear what caused the failure, and users can easily recreate. My 2c

----- Original Message -----

Thanks for looking at that PR @ixdy.
Thats a valid concern.
For a short time, after we enable v1beta3 and before we deprecate v1beta1, we
need to test both the API versions.
Looks like /hack/test-go takes about 2 minutes right now to run on travis. So
we will be increasing its run time by 2 minutes by running it twice.
We can try it and see if travis behaves fine.
If not, we can pick a random version each time and run the tests for that (a
bit like what our fuzzer tests do now). Maybe we can do that just on travis
and run the tests for both the versions, when it is run locally?
We can even start with that first and see how that works instead.

What do you think?


Reply to this email directly or view it on GitHub:
#1473 (comment)

@nikhiljindal
Copy link
Contributor

@smarterclayton Yes there is an echo at the top of the script that prints the API version for which the tests were run.

I discussed this with @bgrant0607 and he agreed that we can run the tests just once on travis (picking a version at random) and testing both versions locally.
One idea he suggested was that we can choose different APIVersion for each go version, so that we end up testing both api versions in each travis run.

@nikhiljindal
Copy link
Contributor

Fixed by #5687.
We will now test both the versions in test-go and test-integration when run locally.
On travis, we will test "v1beta1" with go 1.4 and "v1beta3" with go 1.3.
You can change the versions being tested using KUBE_TEST_API_VERSIONS env variable.

@bgrant0607 bgrant0607 mentioned this issue Apr 1, 2015
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/reliability area/test area/test-infra priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

9 participants