-
Notifications
You must be signed in to change notification settings - Fork 2
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
RootInlineNode #12
RootInlineNode #12
Conversation
Coverage report
Show new covered files 🐣
Test suite run success140 tests passing in 10 suites. Report generated by 🧪jest coverage report action from e194c67 |
…re/root-inline-node
…e class. Add normalization. Add more tests
src/entities/inline-fragments/specs/InlineTree.integration.spec.ts
Outdated
Show resolved
Hide resolved
const index = 100; | ||
|
||
expect(() => tree.removeText(index)) | ||
.toThrowError(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we also check the error text or type here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a good idea to check for error text as we might change it later. We can use custom error classes, but that's a whole big task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we change the error text, we can also change it in tests. For now any error thrown during execution of removeText()
will lead false-positive test assertion
src/entities/inline-fragments/specs/InlineTree.integration.spec.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Peter Savchenko <specc.dev@gmail.com>
Co-authored-by: Ilya Maroz <37909603+ilyamore88@users.noreply.github.com>
PR adds: