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

Missing actor in generated documentation #3332

Closed
EpicEric opened this issue Oct 8, 2019 · 14 comments · Fixed by #3978
Closed

Missing actor in generated documentation #3332

EpicEric opened this issue Oct 8, 2019 · 14 comments · Fixed by #3978
Labels
needs discussion Needs to be discussed further

Comments

@EpicEric
Copy link
Contributor

EpicEric commented Oct 8, 2019

When building docs for the stdlib, it seems that the file for the _TestRunner actor is not being generated, which results in the following warnings in MkDocs:

$ ponyc packages/stdlib --docs --pass expr
[snip]
$ cd stdlib-docs
$ mkdocs build
INFO    -  Cleaning site directory 
INFO    -  Building documentation to directory: /home/eric/tmp/stdlib-docs/site 
WARNING -  Documentation file 'ponytest-TestHelper.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files. 
WARNING -  Documentation file 'ponytest-_ExclusiveGroup.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files. 
WARNING -  Documentation file 'ponytest-_ExclusiveGroup.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files. 
WARNING -  Documentation file 'ponytest-_Group.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files. 
WARNING -  Documentation file 'ponytest-_Group.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files. 
WARNING -  Documentation file 'ponytest-_SimultaneousGroup.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files. 
WARNING -  Documentation file 'ponytest-_SimultaneousGroup.md' contains a link to 'ponytest-_TestRunner.md' which is not found in the documentation files.

Sure enough, the actor is missing from the ponytest documentation.

@SeanTAllen
Copy link
Member

Are we only generating public documentation but, there's a bug in that?

@EpicEric
Copy link
Contributor Author

EpicEric commented Oct 8, 2019

I don't understand your question.

Every other type (private or public) is generating documentation. This specific actor is not. Here's what the stdlib-docs/docs/ directory looks like:

$ ls -1 stdlib-docs/docs
assert-Assert.md
assert-Fact.md
assert--index.md
backpressure-ApplyReleaseBackpressureAuth.md
backpressure-BackpressureAuth.md
backpressure-Backpressure.md
[...]
ponybench-_TerminalOutput.md
ponytest-_Color.md
ponytest-_ExclusiveGroup.md
ponytest-_Group.md
ponytest--index.md
ponytest-ITest.md
ponytest-PonyTest.md
ponytest-_SimultaneousGroup.md
ponytest-TestHelper.md
ponytest-TestList.md
ponytest-UnitTest.md
process-CapError.md
[...]

For some reason, the ponytest-_TestRunner.md file is missing.


The logs in the OP essentially mean that every link to ponytest-_TestRunner.md will be broken and return a 404. For example, the reference to _TestRunner here.

@SeanTAllen
Copy link
Member

There's an option to only generate public documentation. My question was, are we using that option when it fails?

@EpicEric
Copy link
Contributor Author

EpicEric commented Oct 8, 2019

I'm still not sure I understand you. Sorry.

The command I'm using is the same used to deploy to stdlib.ponylang.io. (reference: .travis_script.bash -> .travis_commands.bash -> Makefile-ponyc). There isn't any other option as far as I can tell.

Also, it only generates warnings. It doesn't stop documentation from being built; it only doesn't know which file to link to and the link will be broken.

@SeanTAllen
Copy link
Member

@EpicEric there's an option to pony to only create docs for public types. looking at the makefile, that option is not being used when creating stdlib docs via the Makefile. See output of ponyc --help for more info about options around generating documentation.

@EpicEric
Copy link
Contributor Author

EpicEric commented Oct 10, 2019

I've opened issue #3340 for that.

This issue is about "why isn't that file being generated" when it should. There's probably some issue with docgen that prevents that file from being generated. I don't know anything about the compiler -- in this case, how it generates documentation -- to figure out why it didn't, eg. if it's related to the generated AST or something else.

@EpicEric EpicEric changed the title Missing actor in stdlib documentation Missing actor in generated documentation Oct 10, 2019
@SeanTAllen
Copy link
Member

So, if the file "isn't used" then it would be excluded. However, it is definitely included in the code that is exercised by the stdlib test program. I suppose that it is possible that it is "optimized away" prior to docgen but I would be shocked if that is the case. The documentation generation pass, as far as I know, happens before any optimization such as dead code removal.

@mfelsche
Copy link
Contributor

The _TestRunner is excluded, because the docgen command excludes tests using some naming conventions. E.g. excluding everything starting with _Test:

https://github.com/ponylang/ponyc/blob/master/src/libponyc/pass/docgen.c#L130 and https://github.com/ponylang/ponyc/blob/master/src/libponyc/pass/docgen.c#L182

I also have some issues with this when building docs for https://github.com/mfelsche/ponycheck but it is not a biggie.

As a long-term goal i would love to have some kind of annotation for excluding types from documentation or forcibly including them (whetever is less verbose), that docgen would use to exclude or include certain types. But that might be just me.

@SeanTAllen
Copy link
Member

@mfelsche I should have known that! I'm the one that put that awful hack in.

@mfelsche
Copy link
Contributor

What is the general feeling about some kind of annotation for that?

If the feeling is positive overall I might start an RFC for that. Maybe we can probe for the feeling with 👍 and 👎 reactions on this comment?

@SeanTAllen
Copy link
Member

I did 👎 because I'd rather revisit just about everything about documentation generation, not because I don't like the idea given the current system.

@SeanTAllen
Copy link
Member

SeanTAllen commented Oct 15, 2019

This could be "temporarily" solved by renaming _TestRunner to _RunnerOfTests. Please don't shoot me.

@jemc
Copy link
Member

jemc commented Oct 15, 2019

To add another possibly unpopular suggestion, I'll say what I've been saying since the start of my involvement with Pony - tests shouldn't be part of the package they are testing, they should be in a subdirectory or nearby companion directory.

This is what I do on all my Pony projects, and it works quite well, despite some limitations that have been cited by some in the past about not being able to write tests that engage directly with private types (this has some workarounds I could explain if desired).

If we did this, we wouldn't need the silly check that looks for _Test at the beginning of types.

@SeanTAllen SeanTAllen added the needs discussion Needs to be discussed further label Sep 5, 2020
@SeanTAllen
Copy link
Member

Given the lack of movement on my part for changing the doc system (ponydoc is still slowly coming along),
I'd like to make the change that I was intending for ponydoc now.

That was to create a \nodoc\ annotation that users can use to exclude code from generated documentation and remove by "is test" hack.

I've brought this up in Zulip here:

https://ponylang.zulipchat.com/#narrow/stream/189952-compiler-discussion/topic/.5Cnodoc.5C.20annotation

SeanTAllen added a commit that referenced this issue Jan 29, 2022
The can be used to control the pony compilers documentation generation,
any structure that can have a docstring (except packages) can be
annotated with \nodoc\ to prevent documentation from being generated.

This replaces a hack I put in years ago to filter out items that
were "for testing" by looking for Test and _Test at the beginning of
the name as well as providing UnitTest or TestList.

All standard library test items have been annotated with \nodoc\ to
keep them continuing to not have documentation generated by our
standard process of creating the standard library documentation site.

ponylang org libraries will need to be updated as well.

Note that the "should we include this package" hack to filter out
packages called "test" and "builtin_test" still exists for the time
being until we have further discussion.

Closes #3332
SeanTAllen added a commit that referenced this issue Jan 29, 2022
The can be used to control the pony compilers documentation generation,
any structure that can have a docstring (except packages) can be
annotated with \nodoc\ to prevent documentation from being generated.

This replaces a hack I put in years ago to filter out items that
were "for testing" by looking for Test and _Test at the beginning of
the name as well as providing UnitTest or TestList.

All standard library test items have been annotated with \nodoc\ to
keep them continuing to not have documentation generated by our
standard process of creating the standard library documentation site.

ponylang org libraries will need to be updated as well.

Note that the "should we include this package" hack to filter out
packages called "test" and "builtin_test" still exists for the time
being until we have further discussion.

Closes #3332
SeanTAllen added a commit that referenced this issue Jan 29, 2022
The can be used to control the pony compilers documentation generation,
any structure that can have a docstring (except packages) can be
annotated with \nodoc\ to prevent documentation from being generated.

This replaces a hack I put in years ago to filter out items that
were "for testing" by looking for Test and _Test at the beginning of
the name as well as providing UnitTest or TestList.

All standard library test items have been annotated with \nodoc\ to
keep them continuing to not have documentation generated by our
standard process of creating the standard library documentation site.

ponylang org libraries will need to be updated as well.

Note that the "should we include this package" hack to filter out
packages called "test" and "builtin_test" still exists for the time
being until we have further discussion.

Closes #3332
SeanTAllen added a commit that referenced this issue Jan 29, 2022
The can be used to control the pony compilers documentation generation,
any structure that can have a docstring (except packages) can be
annotated with \nodoc\ to prevent documentation from being generated.

This replaces a hack I put in years ago to filter out items that
were "for testing" by looking for Test and _Test at the beginning of
the name as well as providing UnitTest or TestList.

All standard library test items have been annotated with \nodoc\ to
keep them continuing to not have documentation generated by our
standard process of creating the standard library documentation site.

ponylang org libraries will need to be updated as well.

Note that the "should we include this package" hack to filter out
packages called "test" and "builtin_test" still exists for the time
being until we have further discussion.

Closes #3332
SeanTAllen added a commit that referenced this issue Feb 1, 2022
The can be used to control the pony compilers documentation generation,
any structure that can have a docstring (except packages) can be
annotated with \nodoc\ to prevent documentation from being generated.

This replaces a hack I put in years ago to filter out items that
were "for testing" by looking for Test and _Test at the beginning of
the name as well as providing UnitTest or TestList.

All standard library test items have been annotated with \nodoc\ to
keep them continuing to not have documentation generated by our
standard process of creating the standard library documentation site.

ponylang org libraries will need to be updated as well.

Note that the "should we include this package" hack to filter out
packages called "test" and "builtin_test" still exists for the time
being until we have further discussion.

Closes #3332
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Needs to be discussed further
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants