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

Improve testing - Using text files that represent object might be able to be improved. #53

Closed
chrislovecnm opened this issue Jul 11, 2020 · 5 comments

Comments

@chrislovecnm
Copy link
Contributor

chrislovecnm commented Jul 11, 2020

Testing against strings that represent objects is a bit too fragile for my preference. I am wondering if

var expected Pod *v1.Pod
cmp.Diff(expectedPod, actualPod)

Would work better. When we update API or the k8s API is updated I do not want to modify a ton of text documents. Also as I an into with Calico, the objects are even modified by the Kubernetes server. I had to remove the Pod annotations added by calico in order to get the e2e testing to pass.

I like it when unit won't compile when APIs are updated. I am uncertain about how to address this well.

I would love feedback on this design.

@chrislovecnm chrislovecnm changed the title Improve testing Improve testing - Using text files might be able to be improved. Jul 11, 2020
@chrislovecnm chrislovecnm changed the title Improve testing - Using text files might be able to be improved. Improve testing - Using text files that represent object might be able to be improved. Jul 11, 2020
@vladdy
Copy link
Contributor

vladdy commented Jul 11, 2020

You don't need to update a lot of text documents with the current implementation. If what you get is expected, the '-update' command line flag for test will do the needed update of fixtures.

The problem with the Diff on the exact object in integration/e2e tests is that you can miss objects you do not expect to be created and it is hard to understand what underlying object is expected or the entire setup of multiple objects looking at the test code and matching it to the API objects. It also does not solve the original cause of your issue - there is no predefined expected configuration of test environment to run against. I experimented with KIND working on this operator and it appears to be an appropriate option in my opinion.

Those are just my thoughts and not something what I insist on.

@chrislovecnm
Copy link
Contributor Author

there is no predefined expected configuration of a test environment to run against

That is gonna get fixed. Great point and opening an issue. And I think that will help with part of this. I am a gke guy and rarely run k8s locally, so hence I ran into this. I opened this issue mostly to write and think about this, as we will probably have to test this against GKE and other on-prem systems to pass testing.

@chrislovecnm
Copy link
Contributor Author

@vladdy on a personal note, I very much appreciate your incite and feedback. You know this project a lot better than I do, and did a ton of work on it. It is in really good shape, and I appreciate what you have done with it. I am lucky to work on this now, and collaborate with you more in the future!

@vladdy
Copy link
Contributor

vladdy commented Jul 11, 2020

Thanks Chris. I would be happy to help with some work here.

@johnrk-zz johnrk-zz added this to To do in Kubernetes Operator via automation Jul 13, 2020
@johnrk-zz johnrk-zz added this to To do in Cockroach Kubernetes Operator via automation Jul 14, 2020
@johnrk-zz johnrk-zz removed this from To do in Kubernetes Operator Jul 14, 2020
@johnrk-zz johnrk-zz moved this from Triage to Backlog in Cockroach Kubernetes Operator Jan 28, 2021
@udnay
Copy link
Collaborator

udnay commented Jun 26, 2021

Closing for now, an reopen if we need it

@udnay udnay closed this as completed Jun 26, 2021
Cockroach Kubernetes Operator automation moved this from Backlog to Done Jun 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants