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

Fix line separators #284

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

daniel-rusu
Copy link
Contributor

@daniel-rusu daniel-rusu commented Jan 20, 2023

Addresses 2 categories of issues:

  1. Make the result writer always use system line separators because combining different types prevented tests from being fixable
  2. Fix all the tests that assert multi-line strings to use the system line separator

Note that we'll continue to see some unrelated failing tests on Windows which test file I/O assertions.

@christophsturm
Copy link
Contributor

what about extracting a receiver function for .trimMargin().replace("\n", System.lineSeparator())?

@robfletcher
Copy link
Owner

I was wondering if it would make sense to have Strikt's assertions be lenient about (or normalize) line separators rather than changing all the assertions

@daniel-rusu
Copy link
Contributor Author

what about extracting a receiver function for .trimMargin().replace("\n", System.lineSeparator())?

I could extract a trimMarginWithSystemLineEndings() extension function as long as I have a place to put it that's accessible by all the modules without exposing it to end-users.

However, note that the trimMargin & trimIndent functions are executed at compile time if they are directly applied at the string declaration site so this would make the tests slightly slower.

@daniel-rusu
Copy link
Contributor Author

daniel-rusu commented Jan 20, 2023

I was wondering if it would make sense to have Strikt's assertions be lenient about (or normalize) line separators rather than changing all the assertions

I think that precise correctness is important for a testing library so I would recommend any sort of leniency to be explicit. Perhaps something like this:

expectThat(description)
  .withLenientLineSeparators()
  .isEqualTo(
    """
        first line
        second line
    """.trimIndent()
  )

One complication with this solution is that any of the other string validations would also need to be modified such as startsWith & endsWith in case the content of the expected value contains line separators

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.

None yet

3 participants