Aggregated TestExecutionListener
orders into SpringTestExecutionListenerOrder
, in order to help third-party integrators when choosing the order for a new listener.
I have done two similar PRs in spring-boot and spring-security projects.
Comment From: snicoll
See also #24844
Comment From: andreisilviudragnea
I have thought more about the changes in this PR. The introduced enum would couple all the listeners together and would hinder a possible future separation of the functionality of spring-test
module in more modules, if this is possible now.
Comment From: sbrannen
Thanks for the PR.
I certainly understand the desire for third-party integrators to be able to look up the order for a given TestExecutionListener
, but I don't think we should provide such a mechanism via a centralized enum. In addition, there is no requirement that a given TestExecutionListener
provide an explicit, static order. Listeners do not have to provide any order value at all, and they may choose to somehow calculate the order programmatically instead of returning a static value.
Excerpt from https://github.com/spring-projects/spring-boot/pull/21133#issuecomment-619571205:
Maybe I overthought this and only extracting each order to a constant on each listener would have been enough.
I also considered that at one point to make the order value statically available via the class reference, but for the above reasons I did not opt to implement that.
An enum or local static constants would only partially help to achieve the ultimate goal of this PR. Specifically, the goal is for TestExecutionListener
implemetors to be able to have their listeners ordered before or after other listeners (some from core Spring Framework, Spring Boot, and Spring Security, and some from other third-party frameworks). Thus, the only robust way to address this need is via some form of relative ordering as proposed by @snicoll in #24844.
Thus, after thoughtful consideration, and given that @wilkinsona closed the related PR for Spring Boot (https://github.com/spring-projects/spring-boot/pull/21133#issuecomment-760861928), I am closing this PR.