feat: Unit testing setup restructure #1595
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/core/src/api/tests/clipboard/copy/__snapshots__/text/html/basicBlocks.html
Outdated
Show resolved
Hide resolved
packages/core/src/api/tests/formatConversion/export/getExportTestCases.ts
Outdated
Show resolved
Hide resolved
packages/core/src/api/tests/formatConversion/export/testExport.test.ts
Outdated
Show resolved
Hide resolved
packages/core/src/api/tests/formatConversion/export/testExport.test.ts
Outdated
Show resolved
Hide resolved
...ages/core/src/api/tests/formatConversion/exportParseEquality/testExportParseEquality.test.ts
Outdated
Show resolved
Hide resolved
...ages/core/src/api/tests/formatConversion/exportParseEquality/testExportParseEquality.test.ts
Outdated
Show resolved
Hide resolved
| // Test for verifying that exporting blocks to another format, then importing | ||
| // them back results in the same blocks as the original. Used broadly to ensure | ||
| // that exporting and importing blocks does not result in any data loss. | ||
| const testExportParseEqualityTest = async ( |
There was a problem hiding this comment.
should we not have this for markdown or html? or only for some of those cases?
There was a problem hiding this comment.
also, wdym with Used broadly?
There was a problem hiding this comment.
Added tests for HTML in #1605, though up until now HTML and Markdown wasn't really a concern since the export is lossy, so we didn't really expect the result after export + re-import to be the same as the original blocks.
There was a problem hiding this comment.
Also updated the comment
| <pre><code class="language-python">print("Should parse Python from language- class")</code></pre> | ||
| <pre><code class="language-ruby" data-language="typescript">console.log("Should prioritize TS from data-language over language- class")</code></pre>`, | ||
| }, | ||
| { |
There was a problem hiding this comment.
I get the idea, but I'm not entirely sure if it makes sense markdown / html of these cases should live in the same file
There was a problem hiding this comment.
Changed this so that tests for different formats are in separate arrays, and get their own describe. Imo we don't need to also separate to different files.
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-docx-exporter
@blocknote/xl-multi-column
@blocknote/xl-pdf-exporter
@blocknote/xl-odt-exporter
commit: |
packages/react/src/tests/formatConversion/exportParseEquality/testExportParseEquality.test.ts
Outdated
Show resolved
Hide resolved
|
This looks great. Lot of clean up here. Thanks @matthewlipski for taking this on 💪! |
# Conflicts: # pnpm-lock.yaml # tests/package.json
This PR restructures clipboard and format conversion tests for
@blocknote/coreand@blocknote/react, making all of them use the same design pattern and organizing them in a more sensible way. Also, the tests themselves have been split up, so they are now responsible for one thing instead of testing multiple behaviours at once. These tests have also been moved to thetests/directory in the package root.Moving unit tests to the
tests/directory was a necessary change, as many files containing vitest imports would otherwise need to be exported from@blocknote/core, since they're used in@blocknote/reactThis is an issue for 2 reasons:xyzTestExecutorfiles where vitest is imported to define tests but not run them, which throws the error when the files are exported. The error was preventing the editor from even initializing, and anyway test files shouldn't even be read outside running thetestcommand. After discussing with @nperez0111, the cleanest solution was to move the unit tests to thetests/directory.@blocknote/corefor use in other BlockNote packages, consumers can also access them. Since unit tests are strictly internal, it doesn't make sense for consumers to even be aware of them.I made sure to design the tests in a way that makes sense for all of our unit tests, so that in the future we can also convert the block manipulation tests to use the same pattern, as well as tests in
@blocknote/xl-multi-columnand@blocknote/xl-*-exporterpackages. However, that's out of scope for now.It may also be worth revisiting if we actually need all of our unit tests, as some are probably redundant. However, this is a minor TODO that we can also leave for a future date. The goal of this PR is just to restructure the tests, not to change their scope.
TODO:
Update multi-column package testsUpdate exporter package testsReview what cases need to be tested where, since atm it's largely the same as what we had beforeStyles. (see feat: Less data loss in external HTML #1605)