Skip to content

Add ability to run solo tests #127

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

Closed
ltackmann opened this issue May 26, 2015 · 24 comments
Closed

Add ability to run solo tests #127

ltackmann opened this issue May 26, 2015 · 24 comments

Comments

@ltackmann
Copy link

something like test(solo:true) even better a Test annotation that can be selected from the dart editor to select that only a single test should run

@nex3
Copy link
Member

nex3 commented May 26, 2015

This functionality was very intentionally moved to being handled via command-line flags. Modifying test code to support choosing which tests to run was always a hack; it only existed to work around the inability to consistently parse command-line arguments in tests. It makes it really easy to accidentally check in a change that silently disables a bunch of your tests, and really hard for tooling to select which tests to run.

@nex3 nex3 closed this as completed May 26, 2015
@ltackmann
Copy link
Author

This makes sense. However having to create a new test runner in the dart editor with the correct command line flags evertime I want to execute a single test is a bit tedious. Perhaps creating a @test and @testgroup annotation for tests would allow some IDE tooling to select just a single test or group of tests for execution (like what is done in TestNG and JUnit).

eclipse-runjunittest

@nex3
Copy link
Member

nex3 commented May 27, 2015

It seems way easier for IDE tooling to pass an argument to the test runner than to modify the test code itself.

@yjbanov
Copy link
Contributor

yjbanov commented Jun 17, 2015

Reopening the issue. I'm fine with allowing isolating tests from the command line, but disallowing isolating tests in the test code is a big big loss in developer productivity. Typing strings on the command line means a lot more typing and also either having to copy the text to paste (which involves many more mouse clicks and keystrokes) or memorizing and carefully entering the string/regex yourself. groups existed to allow you to organize related tests in nested groups so you can easily isolate them with fine control over granularity of the functional area of your code you are testing. You could choose multiple groups at different levels and on different branches of your group hierarchy. This was super-useful in unittest (as well as guinness). For example, I could isolate a unit test for a class and a related integration test elsewhere in the test hierarchy. This ability is pretty much destroyed with this change. Groups now are there for visual organization and perhaps a bit of string deduplication, and not much more.

@nex3
Copy link
Member

nex3 commented Jun 17, 2015

@yjbanov This still isn't a feature we plan to add. I understand why it's somewhat easier under some circumstances, but it's also very error-prone and largely redundant with existing functionality. You're welcome to write a wrapper (or eventually a plug-in) that supports this, but there's not going to be direct support.

You could choose multiple groups at different levels and on different branches of your group hierarchy. This was super-useful in unittest (as well as guinness). For example, I could isolate a unit test for a class and a related integration test elsewhere in the test hierarchy. This ability is pretty much destroyed with this change.

You can still do this via something like -n 'test 1|test 2'. If you'd like, feel free to file an issue to add support for multiple -n flags as well.

Groups now are there for visual organization and perhaps a bit of string deduplication, and not much more.

These are legitimately useful things, but they're not the extent of groups' utility. Groups are used for defining configuration or setUp/tearDown for specific sets of tests, and it's pretty easy to select tests in a particular group with -n '^group name'.

@nex3 nex3 closed this as completed Jun 17, 2015
@nex3
Copy link
Member

nex3 commented Jun 18, 2015

I should also mention that you'll be able to roll your own solo testing to some degree once #16 is implemented. If you write something like test(..., tag: "solo") you'll then be able to run only that test (or group, or collection of tests) with -t solo.

@yjbanov
Copy link
Contributor

yjbanov commented Jun 18, 2015

I agree that's a cleaner API, but it still does not provide sufficient ergonomics for fast-paced test-driven workflows such as the one used by Angular and the users of Angular. We prefer to run our tests continuously, so you do not even have to switch away from your code editor. You pick a collection of tests you want to run (via isolating them in the code, where your focus is), save file and the test runner (sensing file system changes) reruns everything automatically. This allows very fast code/test iterations (on the order of seconds per iteration). Having to go to the command-line and to enter flags would slow down the process significantly, as would having to enter tag: "solo".

I suppose we should be able to wrap package:test API into something more ergonomic, but I'm sad to see it not supported out-of-the-box.

BTW, the problem of the risk of submitting isolated tests can be easily solved by the command-line flag to the test runner --enable-isolated-tests, which would be false by default.

@nex3
Copy link
Member

nex3 commented Jun 18, 2015

A continuous test runner like the one you're describing doesn't exist yet; once it does it will likely be in a different package that builds on the test infrastructure rather than being baked into test itself. It should be straightforward for that package to add additional features that fit its particular ergonomical needs. If it can't do it on its own, we can revisit this to see if we can add the necessary support to test without making it worse for people not using the continuous runner.

For now, though, this package only supports batch-mode running. In batch mode a combination of -n and tagging works very well—it doesn't accidentally disable tests or require an additional option, and tagging produces sets of tests to focus on that are persistent over time.

@pq
Copy link
Member

pq commented Sep 23, 2015

Re-opening to give this yet another nudge. I understand the principle behind the choice not to implement this but in practice it's a CONSTANT drag. I'm w/ @yjbanov on the plea for better ergonomics out of the box.

@pq pq reopened this Sep 23, 2015
@nex3
Copy link
Member

nex3 commented Sep 23, 2015

This is still not something we plan to support directly (although again, it will be possible to do something very similar with #16). This isn't a feature that's supported in any other solely-batch-mode test runner that I know of—every other language ecosystem (other than JS which uses a live test runner) is very content with passing the name of the test or using tagging.

It's also not something that we can support in a way that matches user expectations. The test runner runs multiple files, and users expect focused tests to work across files—for example, this is how it works in Karma. But a batch-mode test runner can't load all of the test files at once; test used to do so, and it quickly ran out of memory on large applications. This means that when it starts running, it has no idea whether future tests are marked as focused. You'd need to pass a flag that indicates "something will be focused", but at that point you might as well just use tags.

@nex3 nex3 closed this as completed Sep 23, 2015
@yjbanov
Copy link
Contributor

yjbanov commented Oct 9, 2015

This is how it might work. While test(..., tag: "solo") is terrible ergonomically, a wrapper on top of package:test could be created that provides solo_test and solo_group functions that add the tag for you automatically. Your test code would look like this:

// instead of importing 'package:test' you would
import 'package:test_wrapper/test.dart';

main() {
  group('a', () {
    solo_test('should b', () {});
  });
  solo_group('c', () {
    test('should d', () {});
  });
}

Just like ol' times. Then locally you'd run pub run test --tags 'solo' and on the CI server you'd run it without the --tags. So you don't have the risk of accidentally skipping tests on the CI server.

I think package:test should provide these facilities out-of-the box, since they're safe and the demand is high.

@yjbanov yjbanov reopened this Oct 9, 2015
@nex3
Copy link
Member

nex3 commented Oct 9, 2015

This is still not something we plan to support directly. I disagree that tags are terrible ergonomics; of course, that's a judgment call, but as you point out it's very possible for you to create a wrapper that provides the ergonomics you prefer. Including a tagged alias for test and group by default will just add confusion, since they won't work like the old API or like the Jasmine equivalents in that they'll require --tags solo to have any effect.

@nex3 nex3 closed this as completed Oct 9, 2015
@yjbanov
Copy link
Contributor

yjbanov commented Oct 9, 2015

@nex3 would you care to give an update for reasons why not? I think the issue of accidentally excluding tests was addressed, the feature is both highly requested and also completely opt-in. I can buy the "we cannot" argument from #127 (comment), but let's discuss feasibility separately from merit. First, I'd like to hear a good reason for why this is not a good feature.

As for ergonomics, let's allow people who actually use this feature to make the call. I'm guessing as ardent opponent of it, you are not one of them.

@yjbanov yjbanov reopened this Oct 9, 2015
@lrhn
Copy link
Member

lrhn commented Oct 9, 2015

On Fri, Oct 9, 2015 at 2:47 AM, Yegor [email protected] wrote:

This is how it might work. While test(..., tag: "solo") is terrible
ergonomically

Wouldn't it be nice if you could put named parameters anywhere in the
parameter list? Then you could write:
test(tag: "solo", "name of test", () ... )
so it's easy to both write and remove again.

/L

Lasse R.H. Nielsen - [email protected]
'Faith without judgement merely degrades the spirit divine'
Google Denmark ApS - Frederiksborggade 20B, 1 sal - 1360 København K

  • Denmark - CVR nr. 28 86 69 84

@pq
Copy link
Member

pq commented Oct 9, 2015

As for ergonomics, let's allow people who actually use this feature to make the call.

FWIW the lack of solo tests is one of the main reasons I'm NOT pushing the analyzer packages to transition from unittest. Losing the ability to solo is just too much of a step backwards. (But hey, I am a guitar player... ;))

@nex3
Copy link
Member

nex3 commented Oct 9, 2015

@yjbanov Please stop re-opening this issue; if I change my mind, I'll re-open it myself, but until then it's not helping anything.

would you care to give an update for reasons why not?

Adding new features fundamentally comes at a cost, at the very least to complexity and surface area. A new feature isn't inherently good; it must provide enough value to justify its cost.

The feature you're proposing doesn't provide much value at all—moving a parameter from the end of a function call to the name—and the cost is relatively high. It would be inconsistent on several different axes:

  • All other tags will be user-defined and explicitly written out. Having the solo tag in particular be baked into the system and applied implicitly would be strange, and would obscure the actual fact of what the user is doing.
  • All other test configuration is done as optional arguments. Pushing this one bit of configuration into the name instead would be confusing. Moreover, it would muddy the waters for other configuration: users would have to start explicitly remembering that skip is an argument, where right now that's obvious.
  • Test runners that do support focus tests natively don't support them like we would. Users coming to test from unittest or Jasmine have strong expectations for how this feature works—in particular, that the mere presence of a focused test causes no other tests to run. But, as I explained earlier, this isn't possible for test—we'd need users to pass a command-line flag so select the solo tag. Looking like one mode of interaction and behaving like another is a recipe for user confusion and frustration.

As for ergonomics, let's allow people who actually use this feature to make the call. I'm guessing as ardent opponent of it, you are not one of them.

I used solo_test all the time during the two and a half years that unittest was the framework of choice. It took me a bit to get used to passing test names on the command-line again, but now I find it much easier and more flexible. There's a good reason it's how every batch-mode test runner works.

@nex3 nex3 closed this as completed Oct 9, 2015
@pq
Copy link
Member

pq commented Oct 9, 2015

I used solo_test all the time during the two and a half years that unittest was the framework of choice. It took me a bit to get used to passing test names on the command-line again

Therein lies the rub (my emphasis). I categorically don't run tests from the command-line when I can from within my editor. The command-line is particularly unappealing when I'm running tests in the debugger (basically all the time when I'm running solo tests).

@yjbanov
Copy link
Contributor

yjbanov commented Oct 9, 2015

[EDIT: added Phil's IDE use-case; grammar]

Ok with discussing it while closed. Let's see if I can summarize the desired qualities of the solution:

  1. Test isolation has to be off by default
  2. Test isolation has to be enabled outside the code that's checked in
  3. Test isolation has to apply globally to match user expectations, even if a runner decides to split the run across isolates/processes
  4. Neither the test runner nor any of core APIs should privilege one way of isolating tests (i.e it should not be baked into the system)

Here's are some additional desired qualities that prompted the creation of this issue:

a. it has to be ergonomic
b. standard (makes it easier to debug tests in other people's packages, which is very frequent in large hermetic code repositories)
c. support continuously running and other non-command-line test runners: Karma, in-browser test runners, in-IDE runners. Generally such runners do not give developer a way to change flags from run to run.
d. familiar to people coming from jasmine and guinness
e. reliable way to locate isolated tests (in order to un-isolate them prior to check-in)

Status quo

pub run test --name {PART OF NAME} is the only solution we have today. It satisfies 1-4 and b. However, it fails to satisfy a, c and d.

#16

#16 would satisfy c, as a developer no longer needs to change command-line args between runs. It's slightly better ergonomically, but far from ideal. Here's are some problems with ergonomics:

  • during peaking test fixing sessions one has to isolate/un-isolate tests very frequently, and , tag: 'solo' involves more typing
  • as @lrhn points out it has to go at the end of the test call, so suppose you search for "should emit rainbows" test description; you then need to scroll down to the end of the test code to isolate it
  • it is too sensitive to formatting: suppose prior to check-in you want to find all isolated tests and remove the tag. Did I type exactly tag: 'solo'? Or was it tag: "solo"? Or tag:'solo'? So it loses e.

#16 also loses b.

I'd like to find one method that satisfies a, b, d and e simultaneously, without losing any of the others.

Solution

#127 (comment) shows one way we can solve d and e. However, it requires a separate package, which means it would not be standard and teams that need to work with many 3rd-party packages need to learn that particular project's conventions. For example, the Angular team needs to fix a myriad of other projects when releasing breaking changes, and it's already pretty hard due to different projects configurations. And we very routinely run people's tests and isolate them for finer grained debugging. This would make things worse.

Ultimate solution: provide solo_test and solo_group as part of package:test as convenience functions. They would not have access to private API or treated specially by the runtime. They have to follow --tags like everybody else. Nobody would be forced to use them. But, they would establish a standard convention. It would be available in every package that uses package:test. So when performing sweeping updates across many packages you only need to know one way to isolate tests. Finally, there's a reliable way to find all isolated tests (solve e), since these functions can be located via "Find Usages" in any IDE and they are not subject to text formatting.

@nex3
Copy link
Member

nex3 commented Oct 13, 2015

a. it has to be ergonomic

Other than a small amount of extra typing, this is subjective. I rarely find myself typing-speed-bound when I'm debugging, so I find consistency a much more important component of ergonomics.

b. standard (makes it easier to debug tests in other people's packages, which is very frequent in large hermetic code repositories)

I agree with this one, but I'd broaden it to "consistent". Your consistency of experience across multiple repositories needs to be balanced against the consistency of peoples' experiences configuring their tests and across test frameworks.

Also, Angular already uses guinness, which wraps and renames basic test methods anyway. I would expect that if standardization across different repositories were crucial, it wouldn't be used. Given that it does, it's an easy place to add your own wrapping functionality.

c. support continuously running and other non-command-line test runners: Karma, in-browser test runners, in-IDE runners. Generally such runners do not give developer a way to change flags from run to run.

As I've mentioned above, the test package currently only has a batch-mode runner, so it doesn't make sense to pre-design for continuous runners or bake in support for primarily-continuous features at a lower level than they need to exist.

I would expect most IDEs to gracefully support this use-case, in the simplest case by allowing arguments to be passed manually to the test runner or by being more advanced and pulling the name of the test automatically. The vast majority of languages' test runners work like test; they work with IDEs somehow, and we can follow suit.

d. familiar to people coming from jasmine and guinness

I consider this something nice to have where possible, but not a requirement. We won't ever be able to make it entirely familiar—the method names, for example, are already set in stone. And explicitly requiring these users to tag their tests will produce less friction anyway, because they won't have the same expectations of how it will work that we won't be able to satisfy.

e. reliable way to locate isolated tests (in order to un-isolate them prior to check-in)

Searching for "solo" will do this in either case.


I just don't find the benefits that come from an additional test method compelling relative to the costs. It's a very small additional amount of work, and if that's still too much Guinness already exists as a place to add fit and fdescribe—which are even less typing and even more consistent with Jasmine.

@filiph
Copy link

filiph commented Oct 29, 2015

Please consider this very real use case that makes this a really bad DevExp decision, imho.

  1. I have tests that need to compile DSL scripts, therefore they need to access the file system — therefore I cannot use pub run test without horrible or impractical hacks (here, here).
  2. So I have to run a part of my tests with dart test/some_test.dart which doesn't recognize -n flags.

My options right now:

  • Skip all tests I don't want to run through "solo" — that's idiotic.
  • Comment out all other tests — less work but still stupid, not to mention dangerous.
  • Just forget about it and run all tests, all the time.

I would strongly prefer something like test("name", () {}, solo: True). Of course, doing something like this should put huge red warnings all over the output, similar to what skip does (it puts orange "~1" when you skip a test). But the option should be there.

@yjbanov
Copy link
Contributor

yjbanov commented Oct 29, 2015

@filiph you will have test("name", () {}, tags: "solo") as soon as I stop pretending that I'm too busy to finish my CL. I've been trying to shift this conversation towards tool support and ergonomics. Tools would require a standard way (or at least a convention) to isolate and run your tests. But perhaps we should open a new issue as this thread has become noisy with arguments soon to become obsolete.

@nex3
Copy link
Member

nex3 commented Oct 29, 2015

@filiph The bugs you're seeing should be fixed at the source, not worked around by adding extra features. In particular:

I have tests that need to compile DSL scripts, therefore they need to access the file system — therefore I cannot use pub run test without horrible or impractical hacks (here, here).

Platform.script is unreliable and will never be reliable—it's too tightly coupled to the specific way a script is invoked, which is an implementation detail of pub and any other system that loads scripts (including test). User code should essentially never touch it.

Luckily, there are ways of locating your package's assets, at least if you aren't using transformers, which it sounds like you aren't. pub run test guarantees that the current working directory will always be the root of your package, and if you don't want to rely on that you can use code like this to locate it as well. Once you have that, you can look in the packages/ directory for other dependencies' assets. This is what tests that need to load assets do today.

This is also going to get a lot simpler soon (hopefully in 1.14). The Isolate.packageConfig (dart-lang/sdk#24022) and Isolate.packageRoot (#19430) getters will allow you to easily locate assets without having to go through the pain of figuring out your own package's location manually.

So I have to run a part of my tests with dart test/some_test.dart which doesn't recognize -n flags.

Running a test file directly with the command-line VM is a shortcut. If you're being forced into it, your code is probably making too many assumptions (in this case about Platform.script). The test runner doesn't officially support code that can't run via pub run test.

It's also worth noting that even if we added a solo_test function or a solo argument, it would just be an alias for a tag. In order to use that tag, you'd still have to pass a command-line parameter, which still wouldn't work with the command-line VM alone.

@lukepighetti
Copy link

What about adding "xtest()" method a-la Jest to quickly disable tests.

@grouma
Copy link
Member

grouma commented Jan 3, 2019

Note that we support the solo argument: https://github.com/dart-lang/test/blob/master/pkgs/test_core/lib/test_core.dart#L135

It is marked as deprecated to help prevent users from checking in solo tests which would negatively impact their test coverage.

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

8 participants