modal-labs / pytest-markdown-docs

Run pytest on markdown code fence blocks
MIT License
57 stars 7 forks source link

Add support for specifying metadata options in MDX comments #29

Closed bunchesofdonald closed 1 month ago

bunchesofdonald commented 2 months ago

Summary

This PR adds support for specifying test metadata in .mdx files using MDX-style comments ({/* pmd-metadata: */}) placed above code blocks. This should support all existing options and allow for easily adding more without having to alter the MDX parsing logic.

Example:

{/* pmd-metadata: notest, fixture:capsys */}
```python
print("hello")
captured = capsys.readouterr()
assert captured.out == "hello\n"

Issue: https://github.com/modal-labs/pytest-markdown-docs/issues/28

bunchesofdonald commented 1 month ago

@freider or @mwaskom could I get a review of this? I'd really love to use this module, but this is a big blocker for that.

freider commented 1 month ago

An alternative to having the mdx context propagate in would be to have another pytest marker/arg "--markdown-docs-syntax=mdx-comments" (I just suggested something similar for the superfences syntax that #26 is adding)

bunchesofdonald commented 1 month ago

An alternative to having the mdx context propagate in would be to have another pytest marker/arg "--markdown-docs-syntax=mdx-comments" (I just suggested something similar for the superfences syntax that #26 is adding)

Do you have a preference? I think having it pick up this style from the fact it's parsing an .mdx file makes sense, but if you feel strongly that having an arg is the way to go that also works for me.

Since I think the context would be just a bit friendlier I'll start on that version, but should be easy-ish to change.

freider commented 1 month ago

An alternative to having the mdx context propagate in would be to have another pytest marker/arg "--markdown-docs-syntax=mdx-comments" (I just suggested something similar for the superfences syntax that #26 is adding)

Do you have a preference? I think having it pick up this style from the fact it's parsing an .mdx file makes sense, but if you feel strongly that having an arg is the way to go that also works for me.

Since I think the context would be just a bit friendlier I'll start on that version, but should be easy-ish to change.

The argument for an arg rather than context is that it's more Mintlify-specific than .mdx-specific, but I can see the point for having it as an always-on-thing for .mdx files anyways, just in case there are other renderers than Mintlify that use code fence info blocks for rendered content. Also could be quite useful to have file extension context available in the block extractor anyways, so go for it :)

bunchesofdonald commented 1 month ago

@freider I've addressed your comments, please have a look at the updated code when you have a moment. Let me know if there are any further adjustments or improvements you'd like me to make.

Thanks again for your guidance!

freider commented 1 month ago

@bunchesofdonald release v0.6.0 includes this patch and is up on pypi now FYI

bunchesofdonald commented 1 month ago

Amazing, thank you so much @freider!