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

Missing Click to see difference support in IntelliJ #254

Open
nielsfalk opened this issue Dec 23, 2021 · 8 comments
Open

Missing Click to see difference support in IntelliJ #254

nielsfalk opened this issue Dec 23, 2021 · 8 comments
Labels
🐛 bug Something isn't working

Comments

@nielsfalk
Copy link

With other assertion Frameworks the logmessage of a failure gets an link to a diff.

org.opentest4j.AssertionFailedError: 
Expected :foo
Actual   :foobar
<Click to see difference>

This is very useful especially with comparing large Json.
I don't get it with strikt. I'd like to have another logmessage, which enables the IntelliJ-Click-to-see-difference-Feature or an IntelliJ-Strikt-Plugin wich does something similar.

@robfletcher
Copy link
Owner

Hmm, that definitely was working at some point. I'll investigate.

@robfletcher robfletcher added the 🐛 bug Something isn't working label Jan 3, 2022
@yuriykulikov
Copy link

+1 this. I am used to Ctrl+D on failing tests to bring up the diff view. It is extremely useful and works with AssertJ. It does't work with Strikt.

@yuriykulikov
Copy link

I think it has something to do with incorrect configuration of org.opentest4j.AssertionFailedError before throwing. I can look into that. Do you accept PRs from strangers?

@yuriykulikov
Copy link

Ok, it seems that IntelliJ expects exactly this to be at the end of the AssertionFailedError.message: expected: <two> but was: <one>.
In this case it adds three more lines to the message including the link to see the diff. In this case the whole message becomes:

expected: <two> but was: <one>
Expected :two
Actual   :one
<Click to see difference>

Prefixes before expected do not interfere with that, but any changes to expected, but was or adding suffixes breaks the IntelliJ feature.

Adding descriptions at front will work

expectThat("one").describedAs("1 is actually 2").isEqualTo("two")

as long as expected: <two> but was: <one> is at the end of the line:

Expect that 1 is actually 2 -> expected: <two> but was: <one>
Expected :two
Actual   :one
<Click to see difference>

Also this will work:

▼ Expect that 1 is actually 2:
  ✗ is equal to "two"
          found "one"
@IntelliJ expected: <two> but was: <one>
Expected :two
Actual   :one
<Click to see difference>

@yuriykulikov
Copy link

yuriykulikov commented Aug 24, 2022

In my opinion, this issue is related to #257 and both issues should be addressed at the same time. JUnit first logs the AssertionFailedError.message, which is used by IntelliJ to show the diff and then prints the stacktrace, which starts with AssertionFailed.toString().

Perhaps adding expected: <two> but was: <one> to the message and changing the AssertionFailed.toString() to produce a singe-line string will solve both issues. For example:

▼ Expect that strings are equal:
  ✗ is equal to "expected"
          found "actual"
=> expected: <expected> but was: <actual>
Expected :expected
Actual   :actual
<Click to see difference>

strikt.internal.opentest4j.AssertionFailed: Expect that strings are equal => expected: <expected> but was: <actual>
	at ... stacktrace ...

where strikt.internal.opentest4j.AssertionFailed: Expect that strings are equal => expected: <expected> but was: <actual> is AssertionFailed.toString() and

▼ Expect that strings are equal:
  ✗ is equal to "expected"
          found "actual"
=> expected: <expected> but was: <actual>

is AssertionFailed.message.

@robfletcher what do you think about this idea?

@robd
Copy link
Contributor

robd commented Sep 9, 2022

@yuriykulikov This sounds great - also really missing intellij integration. Is there anyway to get this behaviour somehow without it being implemented in Strikt (eg extension functions, duplicating some part of the Strikt codebase locally or something?)

@yuriykulikov
Copy link

Hello, @robd

I am using this function:

fun <T> Assertion.Builder<T>.contentEquals(expected: T): Assertion.Builder<T> {
  fun <T> contentEquals(actual: T, expected: T) =
      when {
        actual is ByteArray && expected is ByteArray -> actual.contentEquals(expected)
        actual is BooleanArray && expected is BooleanArray -> actual.contentEquals(expected)
        actual is CharArray && expected is CharArray -> actual.contentEquals(expected)
        actual is ShortArray && expected is ShortArray -> actual.contentEquals(expected)
        actual is IntArray && expected is IntArray -> actual.contentEquals(expected)
        actual is LongArray && expected is LongArray -> actual.contentEquals(expected)
        actual is FloatArray && expected is FloatArray -> actual.contentEquals(expected)
        actual is DoubleArray && expected is DoubleArray -> actual.contentEquals(expected)
        actual is Array<*> && expected is Array<*> -> actual.contentEquals(expected)
        else -> false
      }

  return assert("is equal to %s", expected) { actual ->
    if (actual == expected || contentEquals(actual, expected)) {
      pass()
    } else {
      fail("expected: <$expected> but was: <$subject>")
    }
  }
}

Does the trick for me 😺

@robd
Copy link
Contributor

robd commented Jan 20, 2023

In the end I patched this with a somewhat convoluted workaround. I didn't want to have to change all of my assertions or remember to use a non standard assertion, so I implemented an approach based on intercepting strikts AssertionFailed for all tests and trying a vanilla assertEquals to see if that fails. Seems to work ok so far. Something like this from memory:

  1. Create a TestExecutionExceptionHandler :
class IntellijFix : TestExecutionExceptionHandler {
  override fun handleTestExecutionException(context: ExtensionContext, throwable Throwable) {
    when(throwable) {
      is AssertionFailed -> throw assertionEqualsFailure(throwable) ?: throwable
      else -> throw throwable
    }
  }
  
  private fun assertionEqualsFailure(striktFailure: AssertionFailed) = try {
    assertEquals(striktFailure.expected?.value, striktFailure.actual?.value, striktFailure.message)
    null
  } catch (assertEqualsFailure: AssertionFailedError) { assertEqualsFailure }
}
  1. Load this extension with a Service Loader:
  • Enable service loading with a system property (added in build.gradle)
  • Add java service loader file to META-INF dir in correct location pointing to the extension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants