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

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

Open
mdparker opened this issue Jun 21, 2023 · 0 comments

Comments

@mdparker
Copy link
Member

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

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

CC List

  • moonlightsentinel

Description

Consider:

import std.stdio;

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

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

make publictests

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

Comments

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

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

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.

andrei 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("");
}

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 = "...".

andrei 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.

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.

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:

  • Workaround for issue 22596

dlang/phobos#8309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant