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

Introduce test annotation that will abort failing tests (instead of disabling or excluding them) #4648

Closed
wants to merge 1 commit into from

Conversation

yihtserns
Copy link
Contributor

…and will fail if those they are passing (so you'd know they've been accidentally implemented/fixed).

When running mvn test & the @Failing test failed (as expected):

  • Console log (note the "Skipped: 1"):
[INFO] Running com.fasterxml.jackson.databind.records.RecordUpdate3079Test
[WARNING] Tests run: 2, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.010 s -- in com.fasterxml.jackson.databind.records.RecordUpdate3079Test
  • target/surefire-reports/TEST-com.fasterxml.jackson.databind.records.RecordUpdate3079Test.xml:
<?xml version="1.0" encoding="UTF-8"?>
<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report-3.0.xsd" version="3.0" name="com.fasterxml.jackson.databind.records.RecordUpdate3079Test" time="0.01" tests="2" errors="0" skipped="1" failures="0">
  <...snip>
  <testcase name="testRecordAsPropertyUpdate" classname="com.fasterxml.jackson.databind.records.RecordUpdate3079Test" time="0.001"/>
  <testcase name="testDirectRecordUpdate" classname="com.fasterxml.jackson.databind.records.RecordUpdate3079Test" time="0.0">
    <skipped type="org.opentest4j.TestAbortedException"><![CDATA[org.opentest4j.TestAbortedException: Not implemented/fixed yet
	at com.fasterxml.jackson.databind.testutil.Failing$FailingExtension.handleTestExecutionException(Failing.java:30)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No fallback setter/field defined for creator property 'id' (of `com.fasterxml.jackson.databind.records.RecordUpdate3079Test$IdNameRecord`) (through reference chain: com.fasterxml.jackson.databind.records.RecordUpdate3079Test$IdNameRecord["id"])
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
	at com.fasterxml.jackson.databind.deser.CreatorProperty._reportMissingSetter(CreatorProperty.java:358)
	at com.fasterxml.jackson.databind.deser.CreatorProperty._verifySetter(CreatorProperty.java:341)
	at com.fasterxml.jackson.databind.deser.CreatorProperty.deserializeAndSet(CreatorProperty.java:270)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:273)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:344)
	at com.fasterxml.jackson.databind.ObjectReader._bind(ObjectReader.java:2099)
	at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1249)
	at com.fasterxml.jackson.databind.ObjectMapper.updateValue(ObjectMapper.java:4692)
	at com.fasterxml.jackson.databind.records.RecordUpdate3079Test.testDirectRecordUpdate(RecordUpdate3079Test.java:48)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	... 2 more
]]></skipped>
  </testcase>
</testsuite>
  • target/surefire-reports/com.fasterxml.jackson.databind.records.RecordUpdate3079Test.txt
-------------------------------------------------------------------------------
Test set: com.fasterxml.jackson.databind.records.RecordUpdate3079Test
-------------------------------------------------------------------------------
Tests run: 2, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.010 s -- in com.fasterxml.jackson.databind.records.RecordUpdate3079Test

If the @Failing test started passing for whatever reason (e.g. accidentally got fixed/implemented), it will be made to fail with:

java.lang.RuntimeException: Test that has been failing is now passing!

...so we'd know.

…isabling or excluding them) and will fail if those they are passing (so you'd know they've been accidentally implemented/fixed).
@yihtserns yihtserns closed this Jul 26, 2024
@yihtserns
Copy link
Contributor Author

I created this because I didn't understand how "failing" folders are used, and wanted to make it to require less "human-touch".

Closing because after learning about the process/workflow in #4642, seems like this idea is too simplistic and won't fit how "failing" folders are currently managed/maintained.

@yihtserns yihtserns deleted the failing-test-handler branch July 26, 2024 12:20
@cowtowncoder
Copy link
Member

One thing I forgot to mention: we could create Maven profile for running failing tests more conveniently. The way I run tests is from IDE, but ability to more conveniently run via Maven might be useful.

I guess it all comes down to how these tests were to be used -- for me, they are manually run and I am not sure what automation could achieve since typically they won't start passing without explicit work (tend to be for hard-to-fix issues).

@yawkat
Copy link
Member

yawkat commented Jul 26, 2024

This is similar to @PendingFeature in spock. @PendingFeature marks tests that currently fail as expected, and when those tests start to pass, there is an error (so you can unmark them). I think it's fairly useful.

Maybe there is a junit equivalent.

@cowtowncoder
Copy link
Member

I thought about this bit more and I think that above-mentioned @PendingFeature sounds like a good idea -- as long as the only thing to change is that annotation.
I know some modules (Kotlin I think) use "reverse passing" -- that is, test for failing condition, to make tests "pass", and then failing when feature gets fixed. My only problem with that is if test itself is written to test wrong behavior and needs changing.

So I like that one would instead write test that should pass and it is only annotated as "expect not to pass for now, fail if it does pass".
If there's a way to achieve that with JUnit (5), I'd be +1 for getting that done ASAP.

@cowtowncoder
Copy link
Member

@yihtserns So, wrt above, I think what you are trying to achieve makes sense. Only caveats

  1. I think we should still keep failing tests in specific separate folder/package (even if annotated specifically)
  2. Ideally we'd use something JUnit provides
  3. Ideally tests wouldn't need much more than an annotation (i.e. minimally intrusive)
  4. Would be good if we didn't have to copy lots of code across projects (which is why (2) would be great -- although TBH, this really mostly matters to jackson-databind, number of failing tests in other projects is low)

If you think we could proceed with what is here (my apologies for not digging deep here; I am bit time-limited preparing for going on vacation tomorrow), feel free to re-open.

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.

3 participants