Skip to content
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

Add new shouldContainLineWithString assertion #192

Closed
igorwojda opened this issue Feb 15, 2021 · 7 comments
Closed

Add new shouldContainLineWithString assertion #192

igorwojda opened this issue Feb 15, 2021 · 7 comments

Comments

@igorwojda
Copy link

igorwojda commented Feb 15, 2021

Check if the file contains a given string within any of its lines.
Name is to be decided, but something like:

file shouldContainLineContainingString "abc"
file shouldContainLineWithString "abc"

Something like this:

infix fun File.shouldLineShouldContainString(string: String) =
    this.should("The file '${this.absolutePath}' should have a line containing string \"$string\", but does not") {
        readLines().any { it.contains(string) }
    }
@igorwojda igorwojda changed the title Add new shouldLineShouldContainString assertion Add new shouldContainLineWithString assertion Feb 15, 2021
@MarkusAmshove
Copy link
Owner

MarkusAmshove commented Feb 22, 2021

file shouldContainLineContainingString "abc"
file shouldContainLineWithString "abc"

or shouldHaveLineContaining sounds good for me :-)

@priyaaank
Copy link
Contributor

If no one else is working on this operator, I'd like to get started on it. Before I do, want to make sure if there is any opinion around the implementation or approach. I should have a PR ready in few days otherwise.

@MarkusAmshove
Copy link
Owner

If no one else is working on this operator, I'd like to get started on it. Before I do, want to make sure if there is any opinion around the implementation or approach. I should have a PR ready in few days otherwise.

You can give it a shot!

Implementation wise I think there should be one assertion with one parameter, the line that should be within the file. That assertion uses default charset and System.lineseparator.

Charset and line separator should be optional parameters to either the one assertion or an overload, you're free to decide here :-)

@priyaaank
Copy link
Contributor

Thanks, will keep that in mind. The key aspect I want to confirm is the approach to read the file. It is possible to stream it, however assertions will require special logic for reading. For instance if test_string length is 100 and file content is 10 lines each containing 20 chars, I need to load up 200 chars in memory worst case, to ensure any leading or preceding matches are done. But this is efficient that it can handle large files. The alternate is to load up the content completely but in that case, there is a chance, that it may result in a OOM because of the content size.

Given this is used for testing primarily, is it safe to assume that content can be completely loaded instead of partially? let me know what you think about that.

@MarkusAmshove
Copy link
Owner

My approach would be to load a file completely and wait for someone getting a problem with files that are too large, because I agree that most files would fit into memory.

For files not fitting in memory, a user can still stream the file by hand and use a different assertion on the sequence. They might already be careful when asserting file content because they treat their files different in production code.
If the use case comes up a lot we can add an assertion which streams the file

@priyaaank
Copy link
Contributor

Sounds like a reasonable approach to create this operator. I'll come back when I have something in place. Thanks

@MarkusAmshove
Copy link
Owner

Will be done with 1.69 through #204 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants