Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

Clean up the tests in encoding_test.go #69

Closed
surajssd opened this issue Jun 23, 2017 · 2 comments · Fixed by #180
Closed

Clean up the tests in encoding_test.go #69

surajssd opened this issue Jun 23, 2017 · 2 comments · Fixed by #180

Comments

@surajssd
Copy link
Member

surajssd commented Jun 23, 2017

Right now the tests in this file are specified as fixtures in separate file as oppossed to table driven table where it is easy to map what input maps to the output, here since it is scattered around in files, and the data is assigned to variables which makes it hard on someone writing tests because now they have to spend time on naming unique variable name, which makes no sense.

So suggestion on cleaning it all up and moving to one file which has list of all the tests.

These tests were added in #37
Some more context in #37 (review)

@concaf
Copy link
Collaborator

concaf commented Jun 23, 2017

@surajssd +1, let's refactor this.
So, one problem that I personally faced in OpenCompose, was the fact that the test files became huge, and after a while it was very difficult to track down the test and debug. So, something like one test per file, and having multiple such file struck me like a good option. So, let's tackle this problem in such a way that the number of lines in the *_test.go files does not go crazy, since our input structs are huge.

Another problem was that we did not reuse the tests in OpenCompose in a way that we had to write the same inputs for multiple functions over and over. So, one good approach can be that we have some (not all) such input cases which are consistent from the input to unmarshalling to converting. So right now, we have single_container.go and single_container_app.go and single_container_object.go, so all of these files correspond to the same application "single_container". Similarly, we can have this for wordpress, or something. So, besides the unit tests, we also get a broader view that an entire application is being tested and working fine throughout the generation process.

So, keeping file size and reusability in mind, let's refactor this in a better way, what do you suggest?

@surajssd
Copy link
Member Author

Yeah I am good with reusability need to see how k8s does it for some inspiration

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants