Extensions / Annotations to remind user about unfinished work

[Bukama]

In the past few weeks several issues and PRs came up with ideas for extensions/annotations to "remind" the user a certain test

• is disabled because it fails until a fix / the propper implementation
• works now, but the underlying implementation has mistakes or needs to be refactored / has technical debts)

The following came up:

• Annotation that at test fails from a certain date on (@FailsAt Provide a @FailAt annotation #549) to remind your self to finally fixing/refactoring the underlying code
• Disable a test until a certain date (Create @DisabledUntil #366) or until a certain issue has been solved (Disable tests until issue is resolved #367)
• Annotation to mark a test (and inverting the test result), that the current test fails, because the test is correct, but the implementation has bugs. (@NotWorking Add NotWorking test extension (#551 / #546) #546)

So we came to a point where we write tests cheer, but they do not work (yet) cry. Joke aside all those written above seems to be annotatios/extensios to give the user the possibility to place a reminder at test level which automatically warns you at some point that there is work to be done (cause the test then fails).

At our team meeting this week we decided to create an (this) issue to have global look at these ideas and

• see if we find more use cases which fit into the scenario of "theres work to be done, but we don't / can't do it now - so please JUnit remind me about that"
• review the names of those any maybe find a communal annotatio name to show that all these belong to the same kind of topic

Distinction:

We also have issues / extension to

• disable tests, when other tests fail (Disable tests if other tests failed #467 / Failed test disable other tests #505)
• disable tests, if they have a certain name (Disable Parameterised tests selectively #163)

But I don't see those in the same category, because their goal is to reduce the number of executed / failing tests because unfulfilled dependencies / requirements.

[Bukama]

If anyone comes with a better idea for the issue title I'm open to change it!

[helpermethod]

So this is about the naming / grouping of those annotations?

[beatngu13]

Yes. The names are already expressive I think:

• @DisableUntil
• @FailAt
• @NotWorking

But since they target similar problems, "aligning" the naming scheme might help the user to pick the right annotation.

[Bukama]

So this is about the naming / grouping of those annotations?

Yes and thinking if we find other use cases similar to the ones listed above

[helpermethod]

Maybe something like

@HasToBeFixedAfter -> @DisabledUntil
@HasToBeFixedBefore -> @FailAt
@HasToBeFixed -> @NotWorking

[Michael1993]

I just had a talk with @verhas about this and he brought up an interesting point:
@FailAt and @DisabledUntil are incorrect from an architectural standpoint.

Unit tests are supposed to be independent from external factors - including time.

Let's say you have a library, where you have a couple classes and some tests. Some (one or more) tests are annotated with @DisabledUntil or @FailAt. You create a release. A release is supposed to be stable, i.e.: I can download and build it myself (If I recall correctly maven artifacts have source jars that include all the source code, including tests.). However, because some of your tests have what is essentially an expiration date, suddenly your artifact is not stable - if I download it and try to build it after the expiration date (set in an annotation), the build will fail.

We should include a warning about this in the documentation (at the very least). Alternatively, if we could make it so that these annotations behave differently in a release (how would we do that... no idea) that would be even better.

[helpermethod]

I just had a talk with @verhas about this and he brought up an interesting point:
@FailAt and @DisabledUntil are incorrect from an architectural standpoint.

Unit tests are supposed to be independent from external factors - including time.

Let's say you have a library, where you have a couple classes and some tests. Some (one or more) tests are annotated with @DisabledUntil or @FailAt. You create a release. A release is supposed to be stable, i.e.: I can download and build it myself (If I recall correctly maven artifacts have source jars that include all the source code, including tests.). However, because some of your tests have what is essentially an expiration date, suddenly your artifact is not stable - if I download it and try to build it after the expiration date (set in an annotation), the build will fail.

We should include a warning about this in the documentation (at the very least). Alternatively, if we could make it so that these annotations behave differently in a release (how would we do that... no idea) that would be even better.

That makes perfect sense to me. I guess creating something like a report entry would be a better solution, as you could still react to it.

[beatngu13]

Unit tests are supposed to be independent from external factors - including time.

Tests annotated with @FailAt, @DisableUntil, and the like are not necessarily unit tests. But I agree with your/@verhas' actual message. When checking out a version from the past, e.g., for git bisect, the build should be stable. Or at least it should be easy to make it stable again after the "expiration date".

Via Discord @nipafx suggested a "global option" to make such tests pass and personally I like that idea. Maybe – I'm just thinking out loud, so please bear with me – a property to set the dates for @FailAt/@DisableUntil and/or skip them entirely.

Thoughts?

[Bukama]

Thank you for your thoughts @Michael1993 & @verhas! I share these thoughts and think that it comes down to the question, if we want to support this architecutal "misdesign"? From an architectual point of view I don't. From a developers pov I want (documentation, save code I need for later) and don't want (the programm does not behaivor how it should [in the end]). More and more I come to the point that the suggested annotations/extensions should do something in a unit test which is not the task of the unit test but an task/bug issue in some issue tracker. Either fix an existing bug or the final implementation of a features which is not there / in its final form yet.

I see three ways and all are fine for me, when I also put the aspect of maintence (here and for projects using them) into aspect:

• Implement the annotations in pioneer but place a BIG warnining about the misconcept everywhere
• Implement the annotations in another project under the pioneer umbrella
• Don't implement them as they don't support testing in the architectual / desired way

[beatngu13]

Summary from today's maintainer meeting (with @Michael1993, @Bukama, and myself):

• We are OK with having these annotation in the Pioneer code base
• We suggest to have some kind of usage warning (like Mockito does for e.g. deep stubs)
• We prefer a somewhat similar naming scheme (like @helpermethod suggested)
• We want some global options to enforce reproducible/stable builds (e.g. for git bisect)

To be continued … 😉

[nipafx]

As I see it, this issue has the following open discussion threads...

Non-Reproducibility

@Michael1993 and @verhas are correct that a non-reproducible test suite is a problem. The minimum we can do is to have warnings to that effect. At the moment, only @DisabledUntil can cause that and I've prepared a branch (can't push right now, will do so later) that adds a warning to the documentation and Javadoc and issues a report entry every time a test is temporarily disabled. Beyond that, we should find a way to make the tests reproducible - #478 tracks that.

Naming Scheme

It would be nice to have names that are somewhat similar, but I don't think we should insist on that if it makes the individual names worse. I like @helpermethod's proposal "vertically" (the left column of "HasToBe..." is nice) but not horizontally (I think @DisabledUntil is better than @HasToBeFixedAfter, particularly because of its similarity with the other @Disabled annotations; and @ExpectedToFail, the most likely name for that annotation, better than @HasToBeFixed).

I don't think we can make any specific decision on how to name such extensions now and will have to decide on a case by case basis. Let's keep in mind that similarity to other annotation names is nice and strive for it but not to the point where it prevents better names.

If we absolutely have to align names later because we came up with a great naming scheme, we can do that by duplicating the annotations with a new name and (possibly) deprecating and eventually removing the old ones.

More "JUnit remind me" Cases

Nothing came up in the last few months, but I want to point out #477, which is concerned with a large portion of this space.

[nipafx]

It appears to me that all of these threads are resolved; with the exception of the non-reproducibility warnings, for which I will open a PR soon. If nobody objects in the coming days, I will close this issue with the PR.

If you read this after that happened and think there's more to discuss, feel free to reopen.