Assertj-core: Feature Request: `assertThat(file).doesNotHaveContent("string")`

Created on 5 May 2021  Â·  10Comments  Â·  Source: assertj/assertj-core

Currently it is possible to conveniently check a file's content with hasContent, but not the opposite.

Also I'm regularly bumping into the absence of the possibility to directly analyse files as it is possible for strings, so to have something analogous to assertThat("String").contains & doesNotContain. Of course one can do assertThat(FileUtils.readFileToString(file)) but having syntactic sugar in assertJ to avoid this would be lovely as the brevity and clarity assertJ enables is one of it's main strengths IMHO.
These last two may be a separate feature request, however i wanted to gather general feedback about feasibility first.

Most helpful comment

Fair enough, so adding hasContentContaining and hasContentNotContaining would be enough for you?

Another possibility would be to add a content() method that would be get the content and return string assertions on it, that would avoid an explosion of hasContentXxx methods and provide all string assertions:

Ex:

assertThat(file).content()
                .contains("foo")
                .matches(".*d")
                .endsWith("bar")

I prefer the content() option as I'm sure once you have hasContentContaining you will request hasContentMatching(regex) 😆

All 10 comments

Fair enough, so adding hasContentContaining and hasContentNotContaining would be enough for you?

Another possibility would be to add a content() method that would be get the content and return string assertions on it, that would avoid an explosion of hasContentXxx methods and provide all string assertions:

Ex:

assertThat(file).content()
                .contains("foo")
                .matches(".*d")
                .endsWith("bar")

I prefer the content() option as I'm sure once you have hasContentContaining you will request hasContentMatching(regex) 😆

Hi @joel-costigliola and @Philzen, I agree that adding the content() method is a good idea that we can implement more assertions with this interface. So I would like to work on it and submit a PR shortly if you assign this to me.

@wtd2 go for it, there is some complexity on the soft assertions side of things but don't worry we will take care of it, focus on adding the content() methods and the corresponding tests.

A good reference would be to follow and adapt what we did for AbstractThrowableAssert.getCause() which also changes the object under test.

@joel-costigliola Many thanks for your feedback! I think content() is a very elegant solution, allowing for all the assertions that i've been missing, along with many others.

@wtd2 Would this also consistently work for Paths? :thinking:

Nope, we would have to add content() to path to, would be better to create a different issue though

Hi @joel-costigliola and @Philzen, I am sorry for the late reply since I was really busy this week. Currently, I have completed the basic functions for this feature, but the tests are still. I wonder whether should I submit a draft PR or wait for the tests completed.

@wtd2 nothing criminal about doing a draft PR 😉

@wtd2 no problem at all, we all do this on our spare time and I happens quite frequently that I don't have enough time to.
The best thing you can do is to follow the PR guidelines to ease the code review and provide feedback on the points that really matter.

Nope, we would have to add content() to path too, would be better to create a different issue though

@joel-costigliola Happy to do that, but i wanted to mention a suggestion that i've filed which would basically include that (#2205). Would love to hear what you think about it.

I am sorry for the late reply since I was really busy this week.

@wtd2 Pls don't beat yourself up over this – i'm very grateful for somebody taking this up! The world of assertJ has not missed it until now, and i'm sure it can wait a little longer.

Was this page helpful?
0 / 5 - 0 ratings