dart-lang
@visibleForTesting incorrectly warns when used in a test that is not located in the test/ directory
We organize our tests in different directories, and for tests located under a directory not named 'test' the analyzer incorrectly warns about accessing the a member annotated with @visibleForTesting.
Given this directory structure in a package:
./lib/
./test/
./test_integration/
When the test accessing the annotated member is placed under test/, then the analyzer does not warn.
When the test accessing the annotated member is placed under test_integration/, then the analyzer produces a warning.
This makes this annotation unusable for us.
The documentation states:
Marks a member of a package as only public so that the member can be accessed from the package's tests.
The relevant thing should be whether it is accessed from within a test (in the same package), not the name of the directory the test file is placed in.
The relevant thing should be whether it is accessed from within a test (in the same package), not the name of the directory the test file is placed in.
The name of the directory is how the lint is currently deciding that it is a test. This is consistent with the package layout conventions documented on https://dart.dev/tools/pub/package-layout.
What criteria would you propose that the lint use to determine that a file be considered to be a test?
What criteria would you propose that the lint use to determine that a file be considered to be a test?
I'm not the author, but if I was to change the criteria I'd simply add all _test.dart files (and keep the test folder).
The name of the directory is how the lint is currently deciding that it is a test. This is consistent with the package layout conventions documented on https://dart.dev/tools/pub/package-layout.
For larger projects there can be a need to go beyond the standard directory layout convention, the tooling ought to be reasonably flexible to support that.
What criteria would you propose that the lint use to determine that a file be considered to be a test?
The VSCode Dart plugin has no trouble determining the these test files are test files. Perhaps due to import 'package:test/test.dart'? I'd propose that and / or that the file name ends with _test.dart as @FMorschel suggested.
I think VS Code extension (Dart-Code) only uses _test.dart to know if a file is a test (cc @DanTup to confirm).
I think VS Code extension (Dart-Code) only uses _test.dart to know if a file is a test (cc @DanTup to confirm).
only uses
_test.dartto know if a file is a test
That's also fragile. If you have a test split into multiple libraries, perhaps some code is shared between multiple test files, the helper libraries won't be called ..._test.dart, but they will (likely) be in the test/ directory, and are part of a test.
A better distinction may be if the code use is reachable from a non-test entry point. Not as easy and direct to check, though, having to remember all possible entry points that can include a library.
only uses
_test.dartto know if a file is a testThat's also fragile. If you have a test split into multiple libraries, perhaps some code is shared between multiple test files, the helper libraries won't be called
..._test.dart, but they will (likely) be in thetest/directory, and are part of a test.
That's true, but recognizing _test.dart would be a step forwards.
A better distinction may be if the code use is reachable from a non-test entry point. Not as easy and direct to check, though, having to remember all possible entry points that can include a library.
An option could also be the ability to list the test source directories in analysis_options.yaml.
I think VS Code extension (Dart-Code) only uses
_test.dartto know if a file is a test (cc @DanTup to confirm).
It depends a little on the context. Generally it assumes that tests are files that end with _test.dart and either in test/ or integration_test/. There is a setting dart.allowTestsOutsideTestFolder that ignores the folder restriction though (IIRC some users wanted test files alongside the implementation, but others wanted to be able to create files ending _test.dart in lib/ and bin/ as temporary entry points while testing things, and not have them executed with dart test/flutter test).
Dart-Code is not trying to determine which code is "test code" for the annotation though, so the distinction between test files that aren't entry points doesn't mean much. Mostly we want to know things like "if I press F5 while inside this file, should I run it?". In some places we do now check for a main() function, but this requires Outline data so it's not as straight-forward as just checking the path/filename.
For larger projects there can be a need to go beyond the standard directory layout convention, the tooling ought to be reasonably flexible to support that.
What is in the test_integration/ directory? We also support directories named integration_test/ and testing/ for code that is not tests, but which supports tests, or contains test utilities.
We also support directories named
integration_test/andtesting/...
If those are well supported across all the tools then we should document them.
If those are well supported across all the tools then we should document them.
They are listed on https://dart.dev/tools/pub/package-layout (and the Flutter docs note integration_test), but I'm not sure if you have somewhere else in mind.
It doesn't look like the docs on @visibleForTesting mentions integration_test though:
https://pub.dev/documentation/meta/latest/meta/visibleForTesting-constant.html
They are listed on https://dart.dev/tools/pub/package-layout ...
I don't see a reference to testing on that page, and the reference to integration_test isn't in the structure under the heading "Here's what a complete package (named enchilada) that uses every corner of these guidelines might look like:", though I did find a reference to it by searching for it.
It looks like someone decided that integration_test ought to be supported, possibly because the integration_test package (which is discontinued) used it, but didn't make sure that it was handled everywhere.
It seems like it's a bit of a mess at the moment. What we need is for someone to design a reasonable story for where tests should go (or at least how we identify a test) and then make sure that all of the tools conform to that standard.
and the reference to
integration_testisn't in the structure under the heading "Here's what a complete package (named enchilada) that uses every corner of these guidelines might look like:"
This does seem to be where it's listed:
possibly because the
integration_testpackage (which is discontinued) used it
Based on the text at https://pub.dev/packages/integration_test, package:integration_test was just moved into the Flutter SDK, but does still exist and uses that folder:
https://docs.flutter.dev/testing/integration-tests https://github.com/flutter/flutter/tree/main/packages/integration_test#integration_test
Whether these are the best conventions or need revisiting, I don't know - but I think integration_test is used/documented enough that we should at least ensure it's supported anywhere that it's currently missing in the meantime.
Having integration_tests (and testing) be clearly documented is all well and good!
It would still be valuable to have a more flexible support for the analyzer recognizing it is in fact a test file.
- recognizing _test.dart would be a step forwards
- Potentially add the ability to list the test source directories in analysis_options.yaml
(For context: We have a fairly large multi-faceted test suite with these dirs, which we've named consistently so that they are grouped:)
Potentially add the ability to list the test source directories in analysis_options.yaml
This would make it more complicated for other tools to (for example the VS Code extension would need to parse and know about the format of this file in order to determine if the current file is a test, but we try to keep as of parsing and this kind of knowledge inside the SDK/server).
If the current rules are not sufficient, having a rule of "folders that start with test_" might be better than allowing arbitrary folders to be listed. Although, I'm curious if you considered subfolders (test/contract, test/docker, test/integration) - are there reasons that wouldn't work?
These tests are differentiated on needing different harnesses / setups. They can’t run with the same ’dart test’ invocation. The regular ’test’ folder is reserved for tests that don’t need any external setup (unit tests).
Yes the pattern test_* would work!
I think if you take a step back and look at the intention for this annotation - which is to block public usage of these APIs from regular application or library code - then another simpler option emerges.
What if the hueristic is just to allow usage from all non-public directories - where today at least public means the lib/ and probably bin/ dirs of a package.
This also ends up allowing usage from directories like tool/ etc but I don't think that is particularly problematic.
Oh I like that. I often want to use @visibleForTesting code in little tools (/tool/) or benchmarks, etc.