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

Use an assertion library (e.g. testify) for all exercise tests #2411

Open
junedev opened this issue Aug 6, 2022 · 4 comments
Open

Use an assertion library (e.g. testify) for all exercise tests #2411

junedev opened this issue Aug 6, 2022 · 4 comments
Labels
discussion This is a topic for a longer discussion. The issue can stay open for a while.

Comments

@junedev
Copy link
Member

junedev commented Aug 6, 2022

This can of worms was opened up in Slack discussion and is something that has been on my mind for a while so I want to use this issue to follow up on the discussion.

Question: Do we want to use some external package like testify as assertion library in all the exercise tests?

The test runner can handle external dependencies by "baking them in" when creating the docker image so technically, this is feasible.

Here my thoughts on the matter:

  • I am a big fan of testify. It is widely used and besides the nice helpers for slices etc and the nice diff output for structs, I find that even for simple tests the code is a lot more readable.
  • I find it very annoying that the if statements in the tests in pure Go always force you to write the test "backwards". Instead of stating the condition you want for the test to succeed, you need to phrase the condition under which the test fails.
  • The official Go wiki is often cited by the Go-purists as reference for not using an assertion library: https://github.com/golang/go/wiki/TestComments#assert-libraries I find the arguments in there very weak outside of the context of the Go project itself (to be fare, the wiki is mostly meant for the Go project itself afaik).
    • One argument is that you cannot distinguish between what is t.Error and what is t.Fatal. For testify that does not even apply, it provides assert and require for exactly this use case.
    • Another point is "It also forces the assert package to create a whole new sub-language". Hiding complexity from the user of an API is a core Go philosophy. As long as the interface is simple and produces easy to read code, this should not hold us back imo. (Sidenote: I also don't think there is a performance impact a website user would notice.)
    • Then there is "Assert libraries make it too easy to write imprecise tests" - I don't understand how this is any different (if not worse) when writing tests without a library.
  • When working locally, I think it is ok that the student needs to install a dependency. When they start with the track, they probably need to get Go from the internet anyway and because of the way Go works, the student does not need any special command to download the dependencies for the first exercise and does not need to re-download for each exercise.
  • One could argue there are also quite some real world projects not using an assertion library but imo, a lot of them would actually benefit from using one.

While I personally think, using testify would be the right move, the following things are currently holding me back a bit from fully pushing for it:

  • I don't want to start a fight with potential "purists" in the Exercism Go community that might object this change. (Sidenote: I can see the argument of "its nice to demonstrate that you can write tests without external dependencies in Go" but I feel this could also be accomplished in other ways as well.)
  • Testify is widely used but there are also a lot of critics that rightly say it is a bit bloated. But when looking for less bloated assertion libs, there are a lot of options and no clear pick. Also if the assertion lib is too underpowered, it weakens the point of using one in the first place.
  • There was a mention of testify version 2 so it is unclear for how long version 1 will still be maintained. Also there might be lot of movement in these types of libraries soon-ish because generics might allow to write them better. Overall, this means we run the risk that we might need to switch to another library soon as the industry standard changes.

As @andrerfcsantos mentioned somewhere, there is also the option of only using cmp but imo that would bring the drawbacks of having an external dependency but would not address the general issue of test readability.

Another alternative would be to only use testify in some exercises but I would prefer to keep tests consistently easy to read for students.

@junedev junedev added the discussion This is a topic for a longer discussion. The issue can stay open for a while. label Aug 6, 2022
@NobbZ
Copy link
Member

NobbZ commented Aug 6, 2022

As much as I love (d) testify at work, as much am I opposed using it, as long it isn't the one-stop-shop solution in the community.

As long as there are competitors that are equally used in the wild, using a certain framework here, might cause confusion for students that (have to) use something else at work.

If we stick to gos provided testing "library" we lower the burden, as it is more likely that a coworker can "translate".

@junedev
Copy link
Member Author

junedev commented Aug 7, 2022

I checked what some prominent Go projects are doing, I'll leave my findings here for reference:

  • Hashicorp Consul, Nomad and Vault are heavily tested with testify, Terraform uses stdlib and https://github.com/go-test/deep (I did not check all their other big products)
  • Etcd has testify in ~50 test files
  • Kubernetes has testify in ~300 test files

@andrerfcsantos
Copy link
Member

andrerfcsantos commented Aug 9, 2022

I'm also in favor of using testify. Right now there are a couple of problems with our tests that testify (or a similar library) could help us solve:

  • We have a lot of duplicate functions that are doing basically the same things, either to check if slices are equal, check if a floating number is in a given range to adjust for imprecisions in the representations of floats.
  • It could help make our error messages more uniform and consistent across the track.
  • Might make the tests clearer to understand overall.

Teaching something that is not "standard" is always a concern we must have in mind, but I think I'm ok with it in this case, especially because assertion libraries are just functions at its core. They are not frameworks and they don't force on you a way to write tests at a high level. You are still using standard features like functions starting with Test that take a *testing.T from the standard library and you still use go test. And there's nothing forcing us to use assertion functions if we see they don't help us in a particular case - they are there to help, we can just not use them if they don't. So I don't see assertion libraries as forcing a particular way of testing that is not valuable for the student, I believe a student can still get a pretty good idea of how testing works in Go even if we use an assertion library.

However, I agree there might be a balance to be made here, on one hand, we must be careful to not overuse an assertion library, but on the other hand, we must use it when it makes sense. I don't know exactly where that line lies, but I'd say we should just try and we'll figure that out along the way.

On the argument of popularity, I also think testify is popular enough for that to not be a problem.

@thiagonache
Copy link

thiagonache commented Sep 14, 2022

at least for small texts I still think that cmp is simpler and as beauty as testify if you do a small trick. I did some tests comparing assert.Equal(a, b), cmp.Diff(a, b) and cmp.Diff(strings.Fields(a), strings.Fields(b)) and turns out that I prefer cmp.Diff sometimes and cmp.Diff using strings.Fields for most cases...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This is a topic for a longer discussion. The issue can stay open for a while.
Projects
None yet
Development

No branches or pull requests

4 participants