Skip to content

Add test coverage for DomUtils #33768

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

Closed

Conversation

kunaljani1100
Copy link
Contributor

@kunaljani1100 kunaljani1100 commented Oct 22, 2024

Unit tests have been added for the DomUtils class to test whether all the functions in this class work according to their expected behavior. Since testing for this class was not present tests have been added for this class.

In PR #33752, testing of DomUtils was done using Mocked objects, a modification has been done. Tests have loaded small XML files from the test classpath and assertion has been done on the actual Element.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 22, 2024
@snicoll snicoll self-assigned this Oct 22, 2024
@kunaljani1100

This comment was marked as resolved.

@snicoll

This comment was marked as resolved.

@kunaljani1100

This comment was marked as resolved.

snicoll pushed a commit that referenced this pull request Nov 3, 2024
snicoll added a commit that referenced this pull request Nov 3, 2024
@snicoll snicoll closed this in 438d6de Nov 3, 2024
@snicoll snicoll changed the title Unit testing for DomUtils. Add test coverage for DomUtils Nov 3, 2024
@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 3, 2024
@snicoll snicoll added this to the 6.1.15 milestone Nov 3, 2024
@snicoll
Copy link
Member

snicoll commented Nov 3, 2024

@kunaljani1100 thanks for your contribution. Please see d431267 for the polish commit.

Going back to our previous discussion, unsolicited PRs (i.e. without an issue first to discuss the change) is OK but, as a small team, it needs to be in a good state and I am afraid we're not in a capacity right now to do the back and forth to reach that, which is why I polished it directly. Running the build would have revealed that JUnit assertions is not ok (or look at other tests if you need some inspiration).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants