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

ConsoleLauncher should output diff of expected and actual String values #3139

Open
carolosf opened this issue Jan 30, 2023 · 11 comments · May be fixed by #3397
Open

ConsoleLauncher should output diff of expected and actual String values #3139

carolosf opened this issue Jan 30, 2023 · 11 comments · May be fixed by #3397

Comments

@carolosf
Copy link

carolosf commented Jan 30, 2023

As a developer if I am running JUnit tests using the Console Launcher on the command line if two large strings are found to be different and I get "expected:" "but was:" if I want to see what was different I need to copy the strings and manually diff them.

It would be easier if there was --details flag or another flag (I think another flag is a better option) that would show me a diff between the two strings for actionable results. In most cases using an IDE is suitable but in workflows such as Bazel and CI it would be useful if a diff would be displayed.

Apologies if this is already supported didn't find anything obvious in the docs.

@marcphilipp
Copy link
Member

Potential library to implement this: https://github.com/java-diff-utils/java-diff-utils

@carolosf We've briefly discussed this in our team call today and think this would be feasible since AssertionFailedError from opentest4j carries actual and expected in separate fields.

We'll have to take a closer look first to decide whether --details diff would be the right way of enabling this. Maybe it could even be enabled by default.

@carolosf
Copy link
Author

carolosf commented Feb 17, 2023

@marcphilipp

If it becomes the default behaviour it might break IDEs and plug-ins - which is why I suggested a totally separate flag.

I didn't spend much time on it but I noticed previously that the Bazel plugin for IntelliJ does a regex for "expected but was" and splits the results.

Thanks for discussing though - if implemented it would significantly reduce my debugging time!

@marcphilipp
Copy link
Member

@carolosf IDEs and build tools usually do not run tests via ConsoleLauncher but use the Launcher API directly so this shouldn't be a problem.

@marcphilipp marcphilipp changed the title ConsoleLauncher output --details diff flag ConsoleLauncher should output diff of expected and actual String values May 5, 2023
@marcphilipp
Copy link
Member

Team decision: Change ConsoleLauncher output for test failures to include diff if actual and expected of AssertionFailedError are of type CharSequence.

@marcphilipp
Copy link
Member

@carolosf Would you be interested to work on a PR for this?

@bechte
Copy link
Contributor

bechte commented Jul 4, 2023

Hey @marcphilipp & @carolosf - if ok for you, I'd like to provide a PR for this. Any veto?

@carolosf
Copy link
Author

carolosf commented Jul 4, 2023

@carolosf Would you be interested to work on a PR for this?

Would love to but I don't have any capacity to work on this in the near future right now.

@carolosf
Copy link
Author

carolosf commented Jul 4, 2023

Hey @marcphilipp & @carolosf - if ok for you, I'd like to provide a PR for this. Any veto?

I'm not working on this.

@marcphilipp Anything to add?

@bechte bechte self-assigned this Jul 4, 2023
@marcphilipp
Copy link
Member

@bechte Sure, go ahead

@bechte
Copy link
Contributor

bechte commented Jul 18, 2023

To introduce a new dependency to a library, I assume that I am extending the list in gradle/libs.versions.toml, e.g.:

java-diff-utils = { module = "io.github.java-diff-utils:java-diff-utils", version = "4.12" }

And with that plus running a build, I am able to add the following to the platform console project:

dependencies {
	/*...*/
	implementation(libs.java.diff.utils)
	/*...*/
}

Questions to the team, FYI: @marcphilipp, @sbrannen

  • Is that the current approach we are taking on dependencies?
  • Furthermore, would you like to shadow this dependency?

bechte added a commit that referenced this issue Jul 18, 2023
With this commit, a dependency to https://github.com/java-diff-utils/java-diff-utils is introduced.
java-diff-utils is used to create a diff from the expected and the actual result and report it.

See #3139
bechte added a commit that referenced this issue Jul 18, 2023
With this commit, a dependency to https://github.com/java-diff-utils/java-diff-utils is introduced.
java-diff-utils is used to create a diff from the expected and the actual result and report it.

See #3139
@bechte bechte linked a pull request Jul 18, 2023 that will close this issue
6 tasks
@bechte bechte linked a pull request Jul 18, 2023 that will close this issue
6 tasks
bechte added a commit that referenced this issue Jul 18, 2023
With this commit, a dependency to https://github.com/java-diff-utils/java-diff-utils is introduced.
java-diff-utils is used to create a diff from the expected and the actual result and report it.

See #3139
bechte added a commit that referenced this issue Jul 19, 2023
With this commit, a dependency to https://github.com/java-diff-utils/java-diff-utils is introduced.
java-diff-utils is used to create a diff from the expected and the actual result and report it.

See #3139
@bechte
Copy link
Contributor

bechte commented Jul 21, 2023

I have added some questions within the PR #3397.

@marcphilipp marcphilipp modified the milestones: 5.11 M1, 5.12 Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants