dlangBugzillaToGithub / migration_test

0 stars 0 forks source link

The "publictests" target runs unittests at the top-level namespace so they don't have access to #32

Open dlangBugzillaToGithub opened 2 years ago

dlangBugzillaToGithub commented 2 years ago

andrei (andralex) reported this on 2021-12-13T21:30:45Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=22596

CC List

Description

Consider:

import std.stdio;

template canon(string v) { unittest { writeln(v); } }

void main() { alias x = canon!"abc"; }

The unittest works properly because it has access to v being nested inside the canon template. However, putting this unittest inside phobos and running this will fail:

make publictests

It seems this is because that target uses a unittest extractor that does not handle nested unittests properly.

dlangBugzillaToGithub commented 2 years ago

moonlightsentinel commented on 2021-12-13T22:03:51Z

The example is incomplete, the publictests target only considers documented unit tests (e.g. a leading ///).

dlangBugzillaToGithub commented 2 years ago

moonlightsentinel commented on 2021-12-13T22:18:31Z

A documented unittest as defined above would contribute the following to the documentation:

Example:

writeln(v);

Note that the symbol v is entirely undefined - so the test is failing as expected. A user should be able to execute code shown in a documented unittest by simply adding the appropriate import.

Any unittest that relies on internal details of a module / it's members (which are not accessible when importing said module) should be private. This also applies to the template parameter v.

dlangBugzillaToGithub commented 2 years ago

andrei (@andralex) commented on 2021-12-14T04:39:51Z

(In reply to moonlightsentinel from comment #2)

A documented unittest as defined above would contribute the following to the documentation:

Example:

writeln(v);

Note that the symbol v is entirely undefined - so the test is failing as expected. A user should be able to execute code shown in a documented unittest by simply adding the appropriate import.

Any unittest that relies on internal details of a module / it's members (which are not accessible when importing said module) should be private. This also applies to the template parameter v.

The symbol is not undefined because the documentation is nested inside the canon template, which is parameterized on v.

Maybe I simplified the code too much. Expanding it a bit:

/// Module.

import std.stdio;

/// This is a template parameterized on value v. template canon(string v) { /// This is a function that does something. void fun(string); /// This is a unittest that shows how to use the function unittest { fun(v ~ "!"); } }

void main() { canon!"hello".fun(""); }

Looking at the generated html documentation, the declaration of canon is shown first. Nested within is the example. Does this work properly with make publictests?

dlangBugzillaToGithub commented 2 years ago

moonlightsentinel commented on 2021-12-14T12:36:27Z

(In reply to Andrei Alexandrescu from comment #3)

The symbol is not undefined because the documentation is nested inside the canon template, which is parameterized on v.

On a different page, not defined alongside the example.

Also ignores the fact that a documented unittests contributes an executable! example (backed by run.dlang.io) to the offical documentation. Accepting such tests would mean that examples would be broken unless modified by the user.

Does this work properly with make publictests?

No, it would fail as expected. The definition / value of v is unknown to the user who run this example on the website / in his code.


Such tests should probably provide a declaration of v with a default value. This could probably be realized via tooling s.t. phobos can test different versions while the website examples include an explicit enum v = "...".

dlangBugzillaToGithub commented 2 years ago

andrei (@andralex) commented on 2021-12-14T15:12:13Z

(In reply to moonlightsentinel from comment #4)

(In reply to Andrei Alexandrescu from comment #3)

The symbol is not undefined because the documentation is nested inside the canon template, which is parameterized on v.

On a different page, not defined alongside the example.

Also ignores the fact that a documented unittests contributes an executable! example (backed by run.dlang.io) to the offical documentation. Accepting such tests would mean that examples would be broken unless modified by the user.

Allow me to repeat to make sure I understand correctly: no documentation unittest nested inside a template in phobos could ever access the template parameters. The reason is that those unittests are transformed into runnable examples on the website, which are at the top level (hence have no access to the enclosing template's arguments).

That makes sense, I assume it's documented somewhere.

However I see it more of a limitation of the way we go about runnable unittests than a "nothing to be seen here" situation.

Does this work properly with make publictests?

No, it would fail as expected. The definition / value of v is unknown to the user who run this example on the website / in his code.


Such tests should probably provide a declaration of v with a default value. This could probably be realized via tooling s.t. phobos can test different versions while the website examples include an explicit enum v = "...".

Well the problem is a tad deeper. The tests need to be run for all versions of the standard, that's the whole point of putting them in there. Having them check all versions of the standard is great leverage. That we can't do that is a serious issue.

Let's leave this open in search for a good solution.

dlangBugzillaToGithub commented 2 years ago

moonlightsentinel commented on 2021-12-14T18:31:55Z

(In reply to Andrei Alexandrescu from comment #5)

Allow me to repeat to make sure I understand correctly: no documentation unittest nested inside a template in phobos could ever access the template parameters. The reason is that those unittests are transformed into runnable examples on the website, which are at the top level (hence have no access to the enclosing template's arguments).

That makes sense, I assume it's documented somewhere.

Indeed, but note that this restriction also applies to aggregates for the same reason (=> example that can be copy + pasted without additional requirements).

However I see it more of a limitation of the way we go about runnable unittests than a "nothing to be seen here" situation.

[...]

Well the problem is a tad deeper. The tests need to be run for all versions of the standard, that's the whole point of putting them in there. Having them check all versions of the standard is great leverage. That we can't do that is a serious issue.

The real problem here seems to be a different understanding of documented unittests. IMO a documented unittests should be an example that demonstrates the usage of a function/template/... in the most simple way possible without any strings attached.

Your description matches a great test but doesn't sound like a good example.

Let's leave this open in search for a good solution.

Providing a default version as outline above could be an acceptable compromise.

dlangBugzillaToGithub commented 2 years ago

dlang-bot commented on 2021-12-15T03:00:48Z

@andralex updated dlang/phobos pull request #8309 "[WIP] Proof of concept for versioning the standard library" mentioning this issue:

https://github.com/dlang/phobos/pull/8309