Skip to content

Conversation

@eshabakhov
Copy link
Contributor

@eshabakhov eshabakhov commented Oct 30, 2025

Pull request solves #1435

@eshabakhov eshabakhov changed the title test: enhance TkHtml test coverage and readability enhance TkHtml test coverage and readability Oct 30, 2025
@eshabakhov eshabakhov requested a review from sopronyr November 10, 2025 14:47
@eshabakhov
Copy link
Contributor Author

@sopronyr did I answer your question? check pls PR

@eshabakhov
Copy link
Contributor Author

@yegor256 please, check the PR

() -> {
MatcherAssert.assertThat(
"Must reject null byte array body",
new RsPrint(new TkHtml(body.getBytes()).act(new RqFake())),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eshabakhov what this indentation is for? Also, in all other tests. Looks wrong to me. I wonder how Qulice didn't catch this.

@eshabakhov
Copy link
Contributor Author

@yegor256 fixed, check please

* Test case for {@link TkHtml with empty body}.
* @since 0.10
*/
final class TkHtmlEmptyTest {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eshabakhov
Copy link
Contributor Author

@yegor256 thanks for the article!
I thought it would be preferable to use fewer methods in tests so that qulice wouldn't complain.

Fixed, check please. Is it correct to check for null in tests like this?


@Test
void endsTextResponseBodyWithHtmlTag() throws Exception {
final String body = "<html><body>Hello comrade!/body></html>";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eshabakhov
Copy link
Contributor Author

eshabakhov commented Dec 13, 2025

@yegor256 hmm, okay.

Fixed, please check

@yegor256
Copy link
Owner

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Dec 13, 2025

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here.

@rultor rultor merged commit a04420f into yegor256:master Dec 13, 2025
15 checks passed
@rultor
Copy link
Collaborator

rultor commented Dec 13, 2025

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 11min).

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

Successfully merging this pull request may close these issues.

4 participants