tendersearls/tldr

Misc thoughts and feedback

sandstrom opened this issue ยท 2 comments

Very interesting library! ๐Ÿ˜„

A few thoughts:

  • You should open the "Discussions" tab on the repo, for questions etc. Also probably disable 'Project' if you don't use it.
  • Is there a reason why we cannot continue using MiniTest? I thought this was the runner, and that test definition wouldn't necessarily be tied to the choice of runner.
  • class MathTest < TLDR looks weird to me, I would have preferred class MathTest < TLDR::Test or class MathTest < TLDR::TestCase.
  • You should clarify if you still think the tool is nice even if only ~10% of the suit gets run on average, or if you think that's a flaw that your tool will "work against". Basically, I think for us we'll never get it to run in 1.8, but we'd be happy with never running the whole suit locally.
  • Maybe your tooling could somehow cleverly build up a database (based on code-coverage methodologies) and map files to tests, such that it would know which tests are important to run after certain source code files have been changed. I saw you're doing timestamp priorities already, but this would be next-level.

Thanks for the note!

  • You should open the "Discussions" tab on the repo, for questions etc. Also probably disable 'Project' if you don't use it.

I am happy to do half of this and disable the Project tab. I don't think either Aaron nor myself have the energy to monitor and moderate discussions

  • Is there a reason why we cannot continue using MiniTest? I thought this was the runner, and that test definition wouldn't necessarily be tied to the choice of runner.

The simplest reason is that we didn't want the dependency on Minitest and a couple of the things that make Minitest slow (like how it resolves plugins by searching every directory in the load path) would be hard to avoid if this were simply a runner.

That said, I've totally just removed gem "minitest" from a couple codebases and then did something like module Minitest; class Test < TLDR; end; end in my test helper to run a Minitest suite through TLDR instead. A shim like that would be a great feature

  • class MathTest < TLDR looks weird to me, I would have preferred class MathTest < TLDR::Test or class MathTest < TLDR::TestCase.

TLDR is fewer keystrokes and we didn't have time for the extra keystrokes

  • You should clarify if you still think the tool is nice even if only ~10% of the suit gets run on average, or if you think that's a flaw that your tool will "work against". Basically, I think for us we'll never get it to run in 1.8, but we'd be happy with never running the whole suit locally.

We try to articulate this in the README and my blog post but I think this is all just sufficiently weird that there's a certain amount of "you just gotta try it" for it to make sense.

We don't make it clear enough that entire suite will run end to end when it runs in CI and we don't do anything to guarantee that the least-frequently-run tests will ever run (just relying on the law of large numbers and random chance), though.

  • Maybe your tooling could somehow cleverly build up a database (based on code-coverage methodologies) and map files to tests, such that it would know which tests are important to run after certain source code files have been changed. I saw you're doing timestamp priorities already, but this would be next-level.

This is something Aaron and I have been batting around for almost a decade now. I was just reminded that he wrote a blog post describing how this might work after one of our discussions about generating differential Coverage back in 2015 and then later on toptal created this crystalball tool as a way to implement that. I agree it would be very neat to have some kind of integration with that, but reinventing all that stuff would be way beyond the scope of the library.

Thanks for your thoughts!

I don't think either Aaron nor myself have the energy to monitor and moderate discussions

You shouldn't monitor or moderate anything, just open the tab ๐Ÿ˜„ So that non-issues (like this issue) could be posted there, instead of as an issue. It's also a "Stack overflow light" for gem users, where they can help each other.

And there is a neat button so that you can 1-click move issues into that tab (for anything that isn't really an issue, great to keep issues clean).

The simplest reason is that we didn't want the dependency on Minitest and a couple of the things that make Minitest slow (like how it resolves plugins by searching every directory in the load path) would be hard to avoid if this were simply a runner.

Maybe an opportunity to upstream perf-improvements to Minitest? That way, you would have less surface to maintain (no need to maintain all the assertions etc).

TLDR is fewer keystrokes and we didn't have time for the extra keystrokes

This answer doesn't make sense to me, but I guess it's a joke.

I was just reminded that he wrote a blog post describing how this might work after one of our discussions about generating differential Coverage back in 2015 and then later on toptal created this crystalball tool as a way to implement that.

Yes, that's exactly what I had in mind.

Thanks for reading and for making an interesting library! We'll try it out and hopefully switch out minitest. I really like the idea!