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

Add an option to treat '_test.dart' files outside of './test' as tests #2160

Closed
maxlapides opened this issue Dec 9, 2019 · 18 comments
Closed
Labels
in testing Relates to test execution of Dart/Flutter tests for end users is bug
Milestone

Comments

@maxlapides
Copy link

As a convention, we are keeping our test files next to our source files instead of in a separate test directory. Now, when we click "Run" above the test in VSCode, it fails:

Could not find an option named "name".

Run 'flutter -h' (or 'flutter <command> -h') for available flutter commands and options.
Exited (64)

You can reproduce this:

  1. Create a new Flutter project
  2. Add a new file lib/main_test.dart:
import 'package:flutter_test/flutter_test.dart';

void main() {
  test('test test', () {
    expect(123, 123);
  });
}
  1. Click "Run" above test
  2. Check Debug Console

Is there any way to get this working? If not, how do you recommend structuring the test directory? We have a very big project and we want to make sure we keep our code organized.

@DanTup DanTup added this to the v3.8.0 milestone Dec 10, 2019
@DanTup DanTup added in testing Relates to test execution of Dart/Flutter tests for end users is bug labels Dec 10, 2019
@DanTup
Copy link
Member

DanTup commented Dec 10, 2019

I think this is because of 4983d93 where we no longer consider something a test unless it's both inside /test and ends with _test.dart. This means we're trying to run this with flutter run instead of flutter test. With that change, the test code-lens links probably should've also been removed.

Of course, that would prevent having tests outside of /test (which sounds like it might not work for you). I'll see if I can come up with a way to handle this without just re-introducing any of the issues mentioned in the changeset above.

@DanTup
Copy link
Member

DanTup commented Dec 10, 2019

Re-reading the other issues, I think this one is exactly the opposite of #2021 (cc @escamoteur) - which wants to have _test files in lib that are not run with the test framework.

While we could make the CodeLens links force test mode, I think the experience should be consistent with just running the "current file". It would be strange to have CodeLens links that consider the file a test, but pressing F5 considers it not-a-test.

I don't know which is more common - people having tests inside of /lib, or people having _test.dart files that aren't tests. Does anyone have any feelings about this?

@maxlapides
Copy link
Author

I'm not sure what reason there is for having a file named _test.dart that is NOT a test file. It seems to me that it is a better assumption that ALL _test.dart files are test files. It would be easier for someone who named a non-test file _test.dart to rename that file than it will be for us to completely reorganize our project to relocate all test files to a /test directory.

@escamoteur
Copy link

I disagree, _test can be a part of a TestApp to test a certain server. using a filename to determine how to handle it is not a good idea. A dedicated test folder makes more sense and is is the way most IDEs handle this.

@DanTup
Copy link
Member

DanTup commented Jan 2, 2020

I'm not sure what reason there is for having a file named _test.dart that is NOT a test file.

I previously thought the same, but I often create files to test issues people report, and ended up with a lot of _test.dart files in my bin folder (since it just felt natural to name them that way - they're scripts for testing things, but not tests).

using a filename to determine how to handle it is not a good idea. A dedicated test folder makes more sense and is is the way most IDEs handle this.

I don't think there's much difference - I don't think either are ideal. However to keep things simple, we don't want users to need to declare up-front that they want to run a test (with pub) versus just running a script (with dart).

I'm thinking maybe we could change it to:

  • Anything inside test/ that ends _test.dart or inside lib/ that ends _test.dart.

This way we wouldn't classify any loose Dart scripts or anything in bin/ as a test, but we would still support tests alongside code in the package. My only concern is that someone created something like lib/flavour_test.dart in a Flutter project it would run as a test which might not be what they intended (they might be trying an alternative entry point to test something).

We could also add an option to allow users to override it, but I've been resistant to that in the past, because once the option is there, people will just use it to "fix" their issue if it's wrong, and the heuristics will not improved (and this will make it feel like more boilerplate people have to paste in to get started).

@maxlapides
Copy link
Author

@DanTup I like this proposal:

Anything inside test/ that ends _test.dart or inside lib/ that ends _test.dart

That will definitely solve the problem for my team.

@DanTup
Copy link
Member

DanTup commented Jan 3, 2020

Oh, I just realised this solution is directly at-odds with #2021. @escamoteur there isn't much info in that issue (looks like we chatted about it, but I can't easily find that). What's the reason for trying to run non-test files with _test.dart from lib? Are they Flutter entry points (as I mentioned above) or something else?

DanTup added a commit that referenced this issue Jan 3, 2020
@maxlapides
Copy link
Author

maxlapides commented Jan 3, 2020

If you run flutter test lib at the command line, Flutter will test all of the files in lib that end with _test.dart. It will not test any files that do not end with _test.dart.

It seems that this convention is being enforced by Flutter itself, so I think it would be correct for the VSCode plugin to enforce the same convention by running any file that ends with _test.dart as a test file.

@escamoteur
Copy link

@DanTup I just think the file_name outside of the Test folder should trigger to run tests. As you self stated above it's not so uncommon to name a file _test in lib

@DanTup
Copy link
Member

DanTup commented Jan 16, 2020

If you run flutter test lib at the command line, Flutter will test all of the files in lib that end with _test.dart. It will not test any files that do not end with _test.dart.

It's slightly different here, because you're explicitly chosen to run flutter test and not flutter run so the intention is clear. In VS Code we try hard to make it so you can just press F5 to run, and we'll use some rules to figure out the most appropriate thing to do.

We could add a way to override the behaviour, but I've tried hard to avoid this up until now for a couple of reasons (including that it's hard to discover and people will probably just use it instead of reporting when the heuristics are bad). Even if we did do this, I'd still like to make the default work for the most common case (though right now I don't know which is most common).

@DanTup DanTup modified the milestones: v3.8.0, v3.9.0 Jan 16, 2020
@maxlapides
Copy link
Author

I totally appreciate that you're trying to avoid configurability.

I believe that the best default is assuming all files that end with _test.dart are test files, regardless of directory structure. Another reason I haven't mentioned yet is the recent change to only run test files within a test directory is blocking my team. There is currently no way to run our tests within VSCode, so we are currently using the command line. However, at least if you're working with a Flutter app, if you name a file _test.dart that you don't intend to be run as a test (as it seems @escamoteur has), there is a workaround to run your project: simply open any other not _test.dart and run that file.

@DanTup
Copy link
Member

DanTup commented Jan 21, 2020

@maxlapides would fixing this for Code Lens/Test Tree entirely solve your issue, or do you also use F5/launch.json to run specific test files too? I think it could be easily solved for Code Lens/Test Tree by forcing test mode for them (it's obvious they are tests).

If you use F5 for running a whole test file (or launch.json) then I think the only option will be to add a way to override the debugger in the launch.json (though we'll still have to pick one of the options for what happens if you hit F5 with no launch.json).

@maxlapides
Copy link
Author

Forcing Code Lens to run test mode would certainly be a big improvement for us! I feel like this would be a pretty uncontroversial change, that would be an overall improvement.

But unless we wrap all of the tests in a file with a group, it would still be difficult to run an entire file of tests, which is a useful thing to do sometimes. So I don't think forcing Code Lens to run test mode is a complete solution, but it would definitely help!

@martipello
Copy link

I'm also having the same issue using code lens I get 'Could not find an option named "name".'. For integration tests you have your tests in a test_driver folder, all tests are in seperate folders and the app_test.dart holds multiple tests, so we don't want any of the sub directories files ending in _test.dart to run only the app_test.dart which holds groups of tests

@DanTup
Copy link
Member

DanTup commented Feb 26, 2020

Sorry for the delay.. I was hoping to come up with something better, but haven't so far.

I'm thinking the simplest way is a setting, something like dart.allowTestsOutsideTestFolder. When set to false (the default) it will assume that _test.dart scripts outside ./test are not tests (I think this is probably most common, though I have no data). When set to true, then _test.dart anywhere will be considered a test.

CodeLens would use this too, so you'd need to enable it to see codeLens for tests outside of ./test.

If it seems like many people are hitting this and unaware of the setting, we could do something basic like a regex over the file (if it ends with _test.dart) and if it looks like it might include tests, give the user a prompt asking which way they want (and persisting that setting so we don't ask again).

You could set the setting in User Settings (if you do this a lot, it'll apply to all projects), but also if you have an open source/shared project doing this, you could commit it in .vscode/settings.json so that anyone opening that project will have it for that window.

How does this sound?

@DanTup DanTup changed the title Can't debug tests in lib Add an option to treat '_test.dart' files outside of './test' as tests Mar 11, 2020
@DanTup
Copy link
Member

DanTup commented Mar 11, 2020

In the next version, the code lens links will disappear from non-test files (so in the original example here, they would be gone). However if you enable "dart.allowTestsOutsideTestFolder": true in your VS Code settings (should work at any level - though remember folder-settings are ignored in a multi-root workspace so they've need to be at user/workspace level) then the code lens links will appear, and also function correctly.

Pressing F5 will also use this too. If you have lib/foo_test.dart open and press F5, it'll be run as a test file if you have this setting enabled, or just run the Flutter app normally if you don't. The default is that this setting is false.

@DanTup DanTup closed this as completed in 2644684 Mar 11, 2020
@martipello
Copy link

martipello commented Mar 30, 2020

sorry where should this setting go? i couldnt see it in the extensions settings? Actually think this isnt available yet right? is it in the 3.9.0 build?

@DanTup
Copy link
Member

DanTup commented Mar 30, 2020

@martipello This hasn't made a stable release yet - though I did just build a beta version that includes it:

https://github.com/Dart-Code/Dart-Code/releases/tag/v3.9.0-beta.1

Full release notes aren't available yet, but should be later today/tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in testing Relates to test execution of Dart/Flutter tests for end users is bug
Projects
None yet
Development

No branches or pull requests

4 participants