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

Add Tests for Doc Examples #184

Merged
merged 2 commits into from
Feb 23, 2024
Merged

Conversation

aspeddro
Copy link
Contributor

@aspeddro aspeddro commented Jan 22, 2024

  • Add a small test script to test docstrings examples
  • Docstring examples must be compiled
  • Test with ReScript v11

@aspeddro aspeddro marked this pull request as ready for review January 22, 2024 01:02
@zth
Copy link
Collaborator

zth commented Jan 22, 2024

Very nice! A few notes/thoughts:

  • I tried it and it's quite slow. I guess a fast win could be to make it run in parallell? Chunked by CPU cores minus one perhaps.
  • Can we name the temp file after the file it was found in instead + what value/type it is attached to?
  • I wonder if using @rescript/tools and the docs extraction is a better base for this than scanning the folders manually. What do you think?
  • Let's think about how we can generalize this in the future too. Maybe this functionality could ship together with @rescript/tools so it's easy to set this up in your own generated docs.
  • Related to the above, a cool future development would be to have it autoinject the generated JS into the docs, if wanted.

An alternative approach that perhaps could solve the above issues could be to:

  • Emit and instantiate a test project like you do now (could reference the project locally in package.json, doesn't need to be packaged up I think)
  • Use @rescript/tools docgen to find all example code. Emit into files in the example project, like: Core__Array__reduce.res for Core__Array.reduce
  • Build the project. This should let ReScript build it all in parallell. All the info we want to show should be in stderr, and we could simply check if the compiler exits successfully or not to see if CI should be stopped.

What do you think?

@aspeddro aspeddro marked this pull request as draft February 11, 2024 03:22
@aspeddro aspeddro marked this pull request as ready for review February 19, 2024 00:09
@aspeddro aspeddro marked this pull request as draft February 19, 2024 06:11
@aspeddro aspeddro marked this pull request as ready for review February 21, 2024 03:06
@aspeddro
Copy link
Contributor Author

aspeddro commented Feb 21, 2024

Ok. This is ready for review.

Two commits here. The first commit f3af423 has the script tests using rescript/tools to extract docstrings.

The new error log:
image

See all errors reported here

The last commit 89b399b fixes all docstrings examples.

Copy link
Collaborator

@zth zth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! We can refactor the script later on if we want, for now this works well.

@zth zth merged commit efd4a8e into rescript-lang:main Feb 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants