Skip to content

Aggregate TestExecutionListener orders #24977

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

Conversation

andreisilviudragnea
Copy link

@andreisilviudragnea andreisilviudragnea commented Apr 26, 2020

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.

Aggregated TestExecutionListener orders into
SpringTestExecutionListenerOrder, in order to help
third-party integrators when choosing the order for
a new listener.
@snicoll
Copy link
Member

snicoll commented Apr 26, 2020

See also #24844

@andreisilviudragnea
Copy link
Author

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.

@sbrannen
Copy link
Member

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 spring-projects/spring-boot#21133 (comment):

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 (spring-projects/spring-boot#21133 (comment)), I am closing this PR.

@sbrannen sbrannen closed this Jan 17, 2021
@sbrannen sbrannen added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 17, 2021
@andreisilviudragnea andreisilviudragnea deleted the aggregate-test-listener-orders branch April 23, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants