OSGI - org.assertj.core.internal is not exposed and can therefor not be used.

[Zegveld]

When using the xmlunit-assertj module in an OSGI environment tests fail because of the use of classes in the org.assert.core.internal package. This package is not marked as exposed and can therefor not be accessed.

The only use for this import seems to be the Failures class. So far I haven't found a way to avoid it so I'm also creating an issue with the assertj-core module to request it to be moved to the util package.

See assertj/assertj#2026

[scordio]

Following what has been mentioned in assertj/assertj#2026:

1. determine whether or not to filter (their own) elements from the exception's stacktrace. (not avoidable)

AbstractAssert#removeCustomAssertRelatedElementsFromStackTraceIfNeeded() is mostly doing what is needed here, but it is private and is restricted to AssertJ package. We could refactor this part in assertj-core and expose it as protected method.

2. create the AssertionError class. (avoidable)

Could by replaced by fail(String).

As AssertJ 2.x is in maintenance mode, would it be an option for XMLUnit to upgrade to AssertJ 3.x, which would also raise the Java requirement to version 8?

[bodewig]

That's a pretty big jump, given that we've put in some extra effort to stay compatible with newer versions of AssertJ without actually depending on a new version.

If we upgraded the AssertJ dependency we'd force all downstream users to upgrade as well.

It might be an option to add an assertj3 module that explicitly depended on AssertJ 3.whatever_is_needed_to_fix_this and allow people to stick to the 2.x compatible version if they want to. I'm doing something similar for NUnit 2/3 over in XMLUnit.NET and have found a way to keep this manageable. Let me think about it for a while.

[joel-costigliola]

My take on this would be for you guys to add assertj3 module as assertj 2.x is not active anymore but if you decide not too, we are happy to release a new version of 2.x with removeCustomAssertRelatedElementsFromStackTraceIfNeeded visibility changed to protected.

[bodewig]

Yes, this probably is the way to go. I'll add a new assertj3 module and will try to ensure we don't need to duplicate too much code.

Right now I haven't actually investigated the problem at hand at all - I'm not deeply familiar with the code here which has largely been written by @krystiankaluzny - but we'll get there :-). In order to keep things simple I'd make the assertj3 module depend on the first AssertJ release that allows us to access removeCustomAssertRelatedElementsFromStackTraceIfNeeded.

People using older versions of AssertJ will have to stick with the xmlunit-assertj module and live outside of OSGi land. Since this has come up for the first time I don't believe we need to make sure the xmlunit-assertj module works in OSGi environments if xmlunit-assertj3 does and thus don't think we really need another AssertJ 2.x release. Thanks for the offer.

[bodewig]

@scordio @joel-costigliola I believe this is a bit more complex - and right now I don't think making removeCustomAssertRelatedElementsFromStackTraceIfNeeded is going to be enough. Maybe we are doing things in an unusual way and should be using different APIs than we do. Let me take a step back.

Of the assertions we offer the ones that compare two pieces of XML are a bit uncommen in that we want to throw a JUnit 4.x'sComparisonFailure if JUnit happens to be available. And we fall back to Failures.instance().failure(String) if it isn't. For this we've decided to implement our own AssertionErrorFactory mimicing AssertJ's ShouldBeEqual. And this is where we use Failures.failure- just like ShouldBeEqual does in the fallback case. We could create the AssertionErrorourselves but would miss the conditional stack-trace scrubbing and stack dumping Failures performs. Using Fail.fail would throw the exception rather than return it.

The second part is that we have introduced a method similar to AbstractAssert.throwAssertionError​ that accepts an AssertionErrorFactory rather than an ErrorMessageFactory so we can use it together with the factory described in the paragraph above. Here we also want to remove XMLUnit internal stack frames. Making removeCustomAssertRelatedElementsFromStackTraceIfNeeded protected is not going to be enough as we don't know whether stack traces should be scrubbed (Failures.instance().isRemoveAssertJRelatedElementsFromStackTrace()) at all.

[Zegveld]

@scordio , @joel-costigliola , @bodewig
Would it suffice to limit the exposure to boolean isElementOfCustomAssert(StackTraceElement stackTraceElement) in AbstractAssert?

in xmlunit it could then be overridden to return the default action or it's own check. The timing of it being called is then handled by Assertj code internal, while the decision to remove the StackTraceElement can be handled by XmlUnit code.

This way the exposure will be limited to a specific behaviour that you want to extend, and not a duplication of the entire framework which decides when to use the behaviour.

[joel-costigliola]

No problem for us to make isElementOfCustomAssert(StackTraceElement stackTraceElement) protected, we can even do a minor release whenever you need it.

[bodewig]

I agree that making isElementOfCustomAssert protected is going to be much better than allowing to override removeCustomAssertRelatedElementsFromStackTraceIfNeeded.

We'd still need access to the stack-trace cleanup logic for custom AssertionErrors. This could for example be via a new AbstractAssert.assertIonError or throwAssertionError method that accepted an AssertionErrorFactory or directly an AssertError and invoked removeAssertJRelatedElementsFromStackTrace and removeCustomAssertRelatedElementsFromStackTraceIfNeeded when needed.

[bodewig]

I was about to work on a PR for a custom AbstractAssert.assertionError when I realized the way to go in the future seems to be AssertionErrorCreator rather than AssertionErrorFactory. failureWithActualExpected might even work for us. In particular we may not even need to use our own creator in the xmlunit-assertj3 (we'd throw a different kind of exception than in xmlunit-assertj, but that may just be fine) - I'll investigate this.

[bodewig]

unfortunately AssertionFailedError's message is way less useful than JUnit 4's ComparisonFailure used to be, it doesn't contain the actual and expected values - which probably is fine when using tooling that understands AssertionFaiiledError. Running our own JUnit 4 tests we'd now lack XML snippet that explainswhere the XML differs - I was forced to include actual and expected inside of the message explicitly, which may create redundancies in JUnit5 environments.

So be it -> ab90c1d

[bodewig]

once isElementOfCustomAssert is protected I can enable the code of 18e329e and xmlunit-assertj3 will no longer use the Failures class,

I still need to refactor a few things, but we should be close to marking this as fixed (and cutting XMLUnit 2.8.1).

[joel-costigliola]

@bodewig If I understand correctly all you need from us is to make isElementOfCustomAssert protected?

[bodewig]

@joel-costigliola this is correct, yes

[joel-costigliola]

@bodewig I just released 3.18.1 with this change, should be in maven central soonish.

[bodewig]

Many thanks @joel-costigliola @scordio and @Zegveld - I've just pushed a change that uses the now protected method.

I'll be pushing a new SNAPSHOT later today so @Zegveld could verify the new xmlunit-assertj3 module works as expected in an OSGi context. There is some documentation cleanup to be done before I cut 2.8.1 but I think I should get there during the coming weekend. And I need to come up with something so we don't get to maintain two copies of very similar code bases, but this can be done post release.

[Zegveld]

Checked out the latest SNAPSHOT from github, looks good for OSGI.

Did find another issue see #205 also the fix for it is #206 .

[bodewig]

great, thank you.

[bodewig]

I've just release XMLUnit 2.8.1 with the new module. Announcements will go out soonish.