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 TOC generation #248

Open
3 tasks
shonfeder opened this issue Jun 24, 2021 · 6 comments
Open
3 tasks

Add tests for TOC generation #248

shonfeder opened this issue Jun 24, 2021 · 6 comments
Assignees

Comments

@shonfeder
Copy link
Collaborator

Followup to #240

  • Testing it properly
  • Add tests
  • Add support for it to the CLI

As per #240 (comment)

@ghost
Copy link

ghost commented Jun 28, 2021

This might be helpful: remark toc tests.

@sonologico sonologico self-assigned this Jul 4, 2021
@sonologico
Copy link
Collaborator

sonologico commented Jul 4, 2021

@shonfeder, I'm thinking of adding support for different types of tests by having different labels at the end of the lines separating the test cases. What do you think of this approach?

```````````````````````````````` example
...
.
...
````````````````````````````````

```````````````````````````````` example-toc
...
.
...
````````````````````````````````

@shonfeder
Copy link
Collaborator Author

@sonologico sorry for the delay!

Would your approach mean altering the text of the current spec.txt used for testing?

If not, I wonder if could just different file names for different kinds of tests?

More generally, I'm wondering if we shouldn't split additional tests into two groups:

  1. Integration tests, that would either use mdx or the cram-style tests used in dune.
  2. Unit tests, that just confirm the expected AST is generated, or that a fragment renders as expected (for which we could use e.q. like ppx_expect if we want crams-style testing).

I don't have a hard opinion here, except that I find the current testing scheme a bit cumbersome to work with, and only justified by the fact that we are presumably working from the original spec text, which is itself left untouched.

@sonologico
Copy link
Collaborator

I was planning on adding more another test files. We already have more besides spec.txt, but actually, I think it's more sensible to have additional tests (besides the spec) using unit tests as you're suggesting. Which testing framework would you prefer? I don't have a preference. I have used alcotest and ounit in the past. Haven't used ppx_expect yet, but I'm open to it.

@shonfeder
Copy link
Collaborator Author

Ok! I'm happy for you to proceed however you see fit, tho also happy to offer opinions where useful :) If you do end up deciding the current testing approach is sufficient and preferable, and worth extending, I wouldn't begrudge the effort.

I in my own projects, I've preferred alcotest over ounit, and qcheck (sometimes on top of alcotest) over all.

ppx_expect would be especially useful if we want to give inline tests that illustrate behavior. As an example:

https://github.com/shonfeder/um-abt/blob/8c2a205ea1cc143b97dfaac7312500ffd93718e2/lib/abt.ml#L286-L359

It would be convenient for cases where we want to actually see the generated HTML, but aren't testing to a pre-given spec.

@sonologico
Copy link
Collaborator

sonologico commented Jul 18, 2021

Played with the idea here: https://github.com/ocaml/omd/blob/sonologico/toc-ppx_expect/src/omd.ml#L29.
I can see this being nice for some lightweight testing.

I think we'll end up needing something like alcotest, so I'm not sure if it's worth having both, but ppx_expect seems to be really nice work with.

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

No branches or pull requests

2 participants