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

Consider using gotestsum #8

Closed
mayankshah1607 opened this issue Jul 6, 2020 · 5 comments
Closed

Consider using gotestsum #8

mayankshah1607 opened this issue Jul 6, 2020 · 5 comments

Comments

@mayankshah1607
Copy link
Contributor

gotestsum can be used as a drop-in replacement for the go test CLI, and we may want to consider using that. However, I don't yet see any obvious advantages over the current workflow:

  • gotestsum is only a CLI and not a testing framework like Ginkgo to be able to replace it
  • It does not, and is not meant to solve our current reliance on the bash script workflow (for running single, or multiple tests, or for ordering the JUnit reports generated by Ginkgo)
  • Ginkgo already provides us with readable output on stdout
  • gotestsum is meant for producing readable outputs while using the standard testing library; which would mean, we must make a big refactor to our existing ginkgo tests. It would then be a copy of the integration tests, in which case it just makes sense to reuse them rather than having them on a separate repo to prevent redundancy.
@alpeb
Copy link
Member

alpeb commented Jul 7, 2020

I beg to disagree 🙂
Keeping linkerd's testing approach makes it easier for new contributors to get started, i.e. not having to learn a different testing library than standard go's. And being able to reuse linkerd's integration tests would be a big plus IMO.
Given the gotestsum wrapper can provide the JUnit XML output that the standard go test lib lacks, what are then the advantages Gingko would give us? BDD might be appealing to some, but I think by its own is not enough reason to introduce a new testing library.

@mayankshah1607
Copy link
Contributor Author

mayankshah1607 commented Jul 8, 2020

Thanks @alpeb. That makes sense to me. However, I still have a few more concerns over using gotestsum.

And being able to reuse linkerd's integration tests would be a big plus IMO.

Just to clear my understanding, are we talking about directly using the integration tests for conformance validation? i.e, we'd have to then move all our conformance stuff into the upstream linkerd2 repo, right? (Because _test.go files cannot provide exportable functions like other packages).

Given the gotestsum wrapper can provide the JUnit XML output that the standard go test lib lacks, what are then the advantages Gingko would give us?

Ginkgo too provides JUnit XML reports (that can later be aggregated by Sonobuoy as a single readable artifact). In fact, we're using this feature in our current approach (See #1). Apart from that, another advantage that Ginkgo provides over gotestsum or the standard testing library is the UX - the outputs on stdout are more nuanced and detailed. Here is an example from one of the conformance test written using Ginkgo:

=== RUN   TestLifecycle
Running Suite: Lifecycle
========================
Random Seed: 1594185960
Will run 1 of 1 specs

 `linkerd install` 
  can install a new control plane
  /home/mayank/Desktop/linkerd2-conformance/tests/lifecycle/specs.go:18
STEP: Installing linkerd control plane with HA: false
STEP: Running pre-installation checks
STEP: Validating `check` output
STEP: Running `linkerd install`
STEP: Applying control plane manifests
STEP: Checking resources in namespace l5d-conformance
STEP: Running post-installation checks
STEP: Validating `check` output

• [SLOW TEST:43.875 seconds]

/home/mayank/Desktop/linkerd2-conformance/tests/lifecycle/specs.go:8
  `linkerd install`
  /home/mayank/Desktop/linkerd2-conformance/tests/lifecycle/specs.go:17
    can install a new control plane
    /home/mayank/Desktop/linkerd2-conformance/tests/lifecycle/specs.go:18
------------------------------

JUnit path was configured: /home/mayank/Desktop/linkerd2-conformance/reports/01-lifecycle.junit.xml

JUnit report was created: /home/mayank/Desktop/linkerd2-conformance/reports/01-lifecycle.junit.xml

Ran 1 of 1 Specs in 43.875 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped

I'd also like to add how Ginkgo makes structuring your tests very easy, intuitive, expressive and hence organized because of various kinds of "containers" it offers - Describe, Context, BeforeEach, AfterEach, It, When, etc (See here). But these may not be strong enough reasons as they're centered around BDD testing.

I am not really sure if this significantly outweights the problem of new contributors having to learn a new testing framework, which is when the std testing library seems like a great option. However, the question we must adress then is how would we re-use the integration tests for conformance validation? We could move conformance validation to the upstream linkerd2 repo, but we may have to re-think the UX, as it would not conform to the config YAML approach that we have planned. To tackle that would mean housing them on a separate repo (like this one) and re-writing those tests with small changes (for our config YAML approach) would introuce some amount of redundancy.

@alpeb
Copy link
Member

alpeb commented Jul 8, 2020

Ah I hadn't considered the impossibility of exporting tests... How much overlap do you think there will be between conformance tests and integration tests? I think that's what's gonna determine whether we'd like to do the conformance tests in the same linkerd2 repo.

@mayankshah1607
Copy link
Contributor Author

mayankshah1607 commented Jul 8, 2020

@alpeb I'd say that the main body of the tests (the logic, assertions, etc.) would mostly be the same. But if we do plan on directly using the integration tests as they are, the only major drawback that I can think of is that the integration tests will not conform to our idea of having the tests be configurable using a YAML file. This would require major refactoring because the same tests now have to be configurable according to the CI as well as a YAML file (the conformance tool).
We also need to consider that there are some tests such as ingress that conformance validation requires, but integration tests do not have.
@Pothulapati WDYT?

@alpeb
Copy link
Member

alpeb commented Jul 9, 2020

Thanks for your considerations @mayankshah1607 🙂
Having this separate as initially intended would make things clearer, and go's inability to export tests should not drive a decision to do otherwise. And I don't wanna stall this project, so please go ahead with the PR.

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

No branches or pull requests

2 participants