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 integration test for API docs based on qiskit-sphinx-theme #715

Closed
Eric-Arellano opened this issue Jan 26, 2024 · 0 comments · Fixed by #960
Closed

Add integration test for API docs based on qiskit-sphinx-theme #715

Eric-Arellano opened this issue Jan 26, 2024 · 0 comments · Fixed by #960

Comments

@Eric-Arellano
Copy link
Collaborator

Eric-Arellano commented Jan 26, 2024

Part of #223.

The designers have found it really useful to see how our app renders the example docs from qiskit-sphinx-theme because they have a comparison to qiskit.org. See #706.

Whereas that PR #706 tried adding the example docs as a legit project, that's not necessary; the HTML doesn't ever change. Instead, we should save the HTML input as testdata and have integration tests for how htmlToMd.ts handles it. We don't need to test the whole pipeline imo, only the Sphinx conversion. This should use snapshot testing (not inline).

When adding this, we should audit which tests can be deleted in htmlToMd.test.ts - those already are more or less integration tests, but some have been replaced by processHtml.test.ts and imo those tests are too noisy.

Bonus: allow previewing the built docs

It'd be really neat if we could look at the example docs in the preview with ./start. To do that, you'd volume mount ./start so that the files are loaded into something like the folder docs/example-api-docs.

@Eric-Arellano Eric-Arellano self-assigned this Mar 4, 2024
@Eric-Arellano Eric-Arellano moved this to In Progress in Docs Planning Mar 4, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 4, 2024
This is a refactor to unblock the new integration test in
#715. We need to test the
conversion pipeline, so its code needs to live in `lib` not `commands`.

I renamed the function `convertHtmlToMarkdown` to instead
`runConversionPipeline` and moved it to `conversionPipeline.ts`. I
didn't want the phrase "html to markdown" because because we already
have `htmlToMd.ts`; this pipeline does way more than convert HTML to
markdown, such as also copying image files and inlining methods into
classes.

This PR also refactors the rest of `updateApiDocs.ts` via helper
functions. A follow up will refactor `conversionPipeline.ts`.
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
Refactor for #715.
Functionality is the same and regenerating provider didn't change.
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
Part of #715. Before, we
assumed a certain folder structure for the conversion pipeline. Now, you
can set up the pipeline to run anywhere, such as a tmpdir.
@Eric-Arellano Eric-Arellano moved this from In Progress to In Review in Docs Planning Mar 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
Closes #715.

This is useful generally for better testing of our conversion pipeline,
as it's our only test that tests the entire conversion pipeline.

It also is useful to allow us to easily compare Sphinx's behavior vs.
our app's behavior, like the PDFs I uploaded in
#947.

--

I didn't document this in the README because we don't talk about testing
in the README currently. Our README is already quite long, so I was
hesitant to add to it since most technical writers don't care about
testing, unlike Jupyter notebook testing that we do document.

Instead, I documented the test file itself. I'm not sure if this is
clear enough.

---------

Co-authored-by: Arnau Casau <47946624+arnaucasau@users.noreply.github.com>
@github-project-automation github-project-automation bot moved this from In Review to Done in Docs Planning Mar 5, 2024
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
This is a refactor to unblock the new integration test in
Qiskit#715. We need to test the
conversion pipeline, so its code needs to live in `lib` not `commands`.

I renamed the function `convertHtmlToMarkdown` to instead
`runConversionPipeline` and moved it to `conversionPipeline.ts`. I
didn't want the phrase "html to markdown" because because we already
have `htmlToMd.ts`; this pipeline does way more than convert HTML to
markdown, such as also copying image files and inlining methods into
classes.

This PR also refactors the rest of `updateApiDocs.ts` via helper
functions. A follow up will refactor `conversionPipeline.ts`.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Refactor for Qiskit#715.
Functionality is the same and regenerating provider didn't change.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
)

Part of Qiskit#715. Before, we
assumed a certain folder structure for the conversion pipeline. Now, you
can set up the pipeline to run anywhere, such as a tmpdir.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Closes Qiskit#715.

This is useful generally for better testing of our conversion pipeline,
as it's our only test that tests the entire conversion pipeline.

It also is useful to allow us to easily compare Sphinx's behavior vs.
our app's behavior, like the PDFs I uploaded in
Qiskit#947.

--

I didn't document this in the README because we don't talk about testing
in the README currently. Our README is already quite long, so I was
hesitant to add to it since most technical writers don't care about
testing, unlike Jupyter notebook testing that we do document.

Instead, I documented the test file itself. I'm not sure if this is
clear enough.

---------

Co-authored-by: Arnau Casau <47946624+arnaucasau@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant