Leonard Brünings opened SPR-16097 and commented
The current ContentResultMatchers
have direct support for plain strings, json, and xml. I would propose to add another that supports the usage of cssQuery
and DOM
operations.
Using https://jsoup.org/ I've created a small POC and it was rather easy to get this example implemented.
This example checks if the csrf token was correctly injected by spring security.
mockMvc.perform(get("/").with(user("user")))
.andExpect(status().is2xxSuccessful())
.andExpect(html(contains("form input[type=hidden][name=_csrf]")));
I don't think that there is a way to do the same with the current available methods, the closest would be a containsString("<input type=\"hidden\" name=\"_csrf\"")
matcher on the body content, and this only works if the value attribute comes after the name attribute. And it still doesn't check if it is part of the form.
The challenge that I see is to have an expressive syntax without performing redundant work, e.g., only parse the body content into a Document
once.
Furthermore, we'd need expressions for singular and multiple results, since cssQuery
can match more than one Element
.
This is just an example what could be done, suggestions welcome.
andExpect(
content().html()
.toMatch(anElement("form input[type=hidden][name=_csrf]").withAttribute("value", matchesPattern(UUID_PATTERN)))
.toMatch(elements("form input").count(5))
)
0 votes, 5 watchers
Comment From: spring-projects-issues
Rossen Stoyanchev commented
Have you considered the HTMLUnit integration?
As for using jsop as you said it's much better to get the content parsed once. At that point it makes sense to keep it separate, i.e. access the response body content via mockMvc.perform(..).andReturn().getResponse()
and then apply assertions.
Comment From: spring-projects-issues
Leonard Brünings commented
I didn't know about the HtmlUnit integration, which is quite nice, however I think this is a bit too much for the simple use-case of checking if some html is present. There is XmlUnit but the content result matchers still have support for xml (node), the same goes for json (jsonassert/jsonunit). What I dislike about HtmlUnit in this context is that, it breaks the consistency when used with plain mockmvc tests, by requiring a different request syntax. IMHO, this is still the realm of a unit test and full integration test.
Comment From: spring-projects-issues
Rossen Stoyanchev commented
IMHO, this is still the realm of a unit test and full integration test.
Did you mean "not a full integration test"? How would you approach writing tests then given a choice between jsoup matchers and the HTMLUnit integration? When would you prefer which and would you use both as part of an overall test suite for an application?
/cc robwinch
Comment From: spring-projects-issues
Leonard Brünings commented
Did you mean "not a full integration test"? Yes
We use WebDriver tests with Geb to write frontend tests, e.g. going to a site and interacting with it, by clicking links/buttons and input data. Sure you can assert that a _csrf
hidden field is there, but that should have been verified beforehand. But the whole test level is on a different level. HTMLUnit goes into the same direction by simulating a browser, and it can also execute JavaScript.
I would use the Jsoup matchers in my normal controller (+view) tests to make sure that the parts of the html I care about in this particular test are present. So I think it is quite valuable to be able to use it like the other matchers in normal MockMvc fashion.
Comment From: spring-projects-issues
Rossen Stoyanchev commented
Okay this could be useful for basic structural assertions on HTML content alongside similar options for XML and JSON. Were you thinking of putting together a PR?
Comment From: spring-projects-issues
Leonard Brünings commented
Yes, I was thinking about creating a PR but wanted to ask first if it would be accepted in the first place. I take your answer as a go ahead, I'll let you know when I've got it ready.
Comment From: spring-projects-issues
Rossen Stoyanchev commented
Great thanks.
Comment From: spring-projects-issues
Leonard Brünings commented
Rossen Stoyanchev I've created a temporary repository where I've put my current implementation https://github.com/leonard84/SPR-16097. It is still a work in progress, but if you already have some feedback on the API let me know.
This test shows how it is intended to be used: MatcherIT
This test shows the error messages that are produced MatcherTest
Comment From: spring-projects-issues
Rossen Stoyanchev commented
I haven't given it a proper look yet, but I wanted to pass on a comment from robwinch:
we might consider trying to use dependencies we already have... rely on whatever htmlunit is using
Comment From: spring-projects-issues
Rob Winch commented
Rossen Stoyanchev I thought I had, but perhaps I didn't pass this along (sorry if that is the case).... I looked into using HtmlUnit instead of jsoup and it really doesn't make sense. The way HtmlUnit is structured, it doesn't lend well for solving this problem. In short, I don't think we can use our existing dependencies and that jsoup is a fine choice.
Comment From: spring-projects-issues
Leonard Brünings commented
Rossen Stoyanchev, Rob Winch I've implemented this feature as far as the API is concerned, it currently supports Hamcrest-style and AssertJ-style assertions. You can see them in the MatcherIT I'd like to get some feedback before I start documenting it all. * Should we leave both Hamcrest-style and AssertJ-style assertions in the API? * Is there anything missing? * Any improvements?
Comment From: spring-projects-issues
Rossen Stoyanchev commented
Leonard Brünings, thanks for putting this together.
-
Why is it necessary to have both of these variants available in the API?
- elements + document(elements())
- eachElement + document(eachElement())
As far as I can see, they're both built on the same Hamcrest Matchers, but the second exposes a more fluent style to chain multiple assertions. Having both overloads the API and creates questions when to use which? The second seems preferable but first can be shorter. It would be better to have one syntax with the benefits of both.
Right now it doesn't read very well when using document repeatedly:
html ->
document -> anElement
document -> eachElement
...
Each new line is not a new document, but a new selector, which points to something like:
html ->
selector -> anElement
selector -> eachElement
...
I didn't experiment in detail with the changes, so I'm only pointing out how it would read better, but the idea is to re-think having two styles in favor of just one.
BTW I did not see any dependency on AssertJ in the main classes. Am I missing anything? My first thought was we can't justify both Hamcrest and AssertJ given that the rest of MockMvc supports only Hamcrest, but after looking closer it seems to require no AssertJ dependencies.
- Looking at resulting classes, it looks a lot like a Hacmcrest integration for JSoup. Except for
HtmlContentMatcher
all others are either a HamcrestMatcher
, or expose static factory methods for Hamcrest matchers, or support the fluent API around the Hamcrest matchers.
Have you considered proposing such classes to be included in https://github.com/jhy/jsoup ? Hamcrest is a widely used Java library, so it seems generally useful to have such an integration. It's not hard to maintain but it would make more sense to that in jsoup.
So then the only class we need in MockMvc is the HtmlContentMatcher
that would live in the o.s.test.web.servlet.result
package, but others don't belong in the same package. The most likely home would be o.s.test.util
but again if this Hamcrest integration could live closer to JSoup then we'd simply consume it from there.
- As a smaller point, there are some classes that probably don't need to be public. I noticed PropertyMatcher, and may be ElementsMatcher and SingleElementMatcher, but I think there are more. It's worth reviewing what is public vs non-public API.
Comment From: spring-projects-issues
Leonard Brünings commented
yes it is not neccessary to have both APIs, I added the fluent-style because I mentioned it in the description after I implemented the more traditional nested Hamcrest-style. I wanted to try both approaches and see which style would be better. And I agree with reducing it to one style, I lean toward keeping the fluent style as it makes it easier and less awkward to assert multiple things.
The document
matcher was left from the hamcrest API and I didn't have a good alias name for it, what do you think of toMatch
.
perfom(get)
andExpect ->
html ->
toMatch -> anElement
toMatch -> eachElement.hasAttribute
...
Regarding AssertJ, I just called the fluent-api Assertj-style assertions since they more closely match Assertj than Hamcrest-style, without actually having a dependency on Assertj.
I created an issue at Jsoup to ask if they would be interested in incorporating the Hamcrest matchers into Jsoup.
Regarding public-private API, you are correct I haven't really made the decision or cleaned up the visibility. What I definitely want is to enable users to write their own matchers if they so choose, but maybe the predicate is sufficient.
Comment From: gavenkoa
There is ready for use for Json: https://github.com/lukas-krecan/JsonUnit#spring
Comment From: benjamin-dreux
This feature is still missing in the current version.
I don't get why this isn't more prioritised.
Sure we have alternative, but what could look as nice as this
mockMvc.perform(get("/contacts"))
.andExpect(status().isOk())
.andExpect(content().contentTypeCompatibleWith("text/html"))
.andExpect(cssPath("input[name='query']").value(""));
End up like this
var result = mockMvc.perform(get("/contacts"))
.andExpect(status().isOk())
.andExpect(content().contentTypeCompatibleWith("text/html"))
.andReturn();
var output = result.getResponse().getContentAsString();
var element = Jsoup.parse(output).selectFirst("input[name='query']");
Assertions.assertEquals("", element.attr("value"));
Which is obviously more complex to the user. I understand that mockMvc was build to serve rest api purpose, but it would be very nice also for testing html based controller also
Comment From: bclozel
@benjamin-dreux with only two upvotes and your comment over the last seven years I don't think there is enough popular demand for such a feature. This is already supported but obviously not first class. Let's close this issue for now and reconsider in the future if there is more demand.