Bug 21462 – Unittests with visibility

Status
NEW
Severity
enhancement
Priority
P4
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2020-12-08T15:41:42Z
Last change time
2024-12-13T19:13:23Z
Assigned to
No Owner
Creator
Bolpat
Moved to GitHub: dmd#19837 →

Comments

Comment #0 by qs.il.paperinik — 2020-12-08T15:41:42Z
PROBLEM: Most unit tests for a function are next to that function in the same module. This means that a unit test might succeed, but the same code wouldn't compile at usage side because of visibility issues: being in the same module, a unit test always has access to private, package, and protected members. SOLUTION: Allow unit tests to state a VisibilityAttribute [1] after the `unittest` keyword. If none is specified, it is the same as public. GRAMMAR: UnitTest: unittest VisibilityAttribute[opt] BlockStatement SEMANTICS: A unit test only has access to members that have the specified visibility or are "more visible" (in the sense that private members are the least visible). To e.g. test private functions, one would need to state the unit test as `unittest private` necessarily. CONSEQUENCES: This means that a non-private unit test usually must import the module it is in. This can be avoided if this is done implicitly which is almost always the right thing to do. The auto-generated import should be omitted if the unit tests first statement is an ImportDeclaration that imports the module or a package it is in. Still, the unit test might need to import other modules or packages globally imported by the module it is in, except public imports. Documentation for unittest VA, unless VA is `public` should have added a comment that this code will only work at a specific context. The text should be specific for every possible VA. The documentation generator should include the auto-generated import. BREAKAGE: Since some existing unit tests would need to either explicitly state `unittest private` or import some modules to compile. Not breaking code (setting the default visibility to `private`) would probably lead to unit tests not being improved in practice. No code is broken silently. Tests are fixed importing relevant stuff or stating `private` explicitly. [1] https://dlang.org/spec/attribute.html#visibility_attributes
Comment #1 by pro.mathias.lang — 2021-07-20T05:29:33Z
If one wants unittest to only test a certain visibility level, one may place the unittest in a different module. Conventions may arise (Tango used `modname_test.d` IIRC). Adding a language feature for this use case seems a bit too much. Additionally, it would most likely "get in the way" of most unittests. The point of a unittest is to be a conformance test at the lowest level. Hence, it is normal for unittests to test internals of a data type, private members or functions, etc... Going back to the problem, the root of your reasoning is that the unittest would not compile at usage site. But the only unittests that should likely compile outside of the module are *documented unittests*. Introducing a check that only non-private members are called in a documented unittest *might* work, but some people (like myself) are writing documented unittests even for internal helpers, so it should be possible to opt-out (or be opt-in). All things considered, this sounds like a good addition to DScanner.
Comment #2 by qs.il.paperinik — 2023-02-22T13:57:45Z
(In reply to Mathias LANG from comment #1) > All things considered, this sounds like a good addition to DScanner. I should have a look at DScanner. > If one wants unittest to only test a certain visibility level, one may place > the unittest in a different module. Conventions may arise (Tango used > `modname_test.d` IIRC). Adding a language feature for this use case seems a > bit too much. People like to have their unittests next to the function. Even in aggregate templates, people jump through hoops and hurdles to get there. Documentation generation is the primary reason, but not the only one. Maintainability is another. > Additionally, it would most likely "get in the way" of most unittests. The > point of a unittest is to be a conformance test at the lowest level. That is the point of *some* unittests. Those would be private unittests. > Hence, it is normal for unittests to test internals of a data type, private members or functions, etc... Depends on how you define normal, I guess. Unittests also make sure a function can actually be used (from outside the module/package, of course) the way it was intended to. > Going back to the problem, the root of your reasoning is that the unittest > would not compile at usage site. But the only unittests that should likely > compile outside of the module are *documented unittests*. > > Introducing a check that only non-private members are called in a documented > unittest *might* work, but some people (like myself) are writing documented > unittests even for internal helpers, so it should be possible to opt-out (or > be opt-in). A possible solution would be: By default, if the unittest is documented, it is a public unittest and private otherwise. You can put `private` on a documented unittest and `public` on an undocumented one to override the default, of course. In an OOP environment, you’d maybe like to make sure a derived class can make use of the protected members as intended and a `protected` unittest could demonstrate that. I just don’t like the idea that semantics of the unittest depend on a (documentation) comment.
Comment #3 by robert.schadek — 2024-12-13T19:13:23Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19837 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB