Since 1.0.5.RELEASE, BlockHound dependency has runtime scope in blockhound-junit-platform pom.xml
simonbasle opened this issue · 7 comments
Adding blockhound-junit-platform
as a dependency used to transitively bring in blockhound
, since up until 1.0.4.RELEASE the scope of blockhound
in blockhound-junit-platform's pom.xml was compile.
Since the move from Gradle 4.10.2 to 6.8, the end-result scope in the pom is runtime.
Users that only imported blockhound-junit-platform
are encouraged to add an explicit dependency to blockhound
as well (sharing the same version number).
I'm inclined to keep it as is, with an upgrade warning in the release notes. The idea here is that as far as I understand we would need to make the blockhound-junit-platform a java-library
with an api
dependency to blockhound for it to expose it as <scope>compile</scope>
in the pom again. Given the nature of the code, it is hard to argue it is a library and harder yet that it exposes "a blockhound API".
Please chime in if you have strong arguments in favor of going back to a compile
scope in future releases.
Examples in the wild (aka github):
- ✅ spring-cloud-gateway declares both dependencies already
- ✅ spring-state-machine as well
- ✅ r2dbc-mssql as well
- ✅ r2dbc-postgresql
⚠️ projects generated byjhipster/generator-jhipster
only declare dependency to theblockhound-junit-platform
⚠️ datastax's java-driver as well⚠️ reactor-netty as well
I double checked with simple projects using both Gradle and Maven that test depending solely on blockhound-junit-platform:1.0.5.RELEASE
works if all one wants to achieve is check that no problematic blocking code is executed as defined by built-in integrations. For example, it flags a Thread.sleep
run inside a Reactor Schedulers.parallel().schedule(...)
. This is because both Gradle and Maven transitively add blockhound
as a runtime dependency of tests in that case.
If one wants to referenc ANY blockhound
API in their tests (such as assertThat(exception).isInstanceOf(BlockingOperationError.class)
for instance), then they'll need to explicitly add the testCompile
dependency to blockhound
.
Oh, that's a major issue :(
I would say we must fix it ASAP, as not only it breaks existing usage, but also makes many publicly available articles about BlockHound invalid. Last but not least, such change must have been discussed in advance, and "we performed a major upgrade of the build tool and didn't notice this" IMO is not a good argument for keeping it as it is.
btw, "projects generated by jhipster/generator-jhipster
" isn't a single project, but many projects that would need to be updated
If one wants to referenc ANY blockhound API in their tests (such as assertThat(exception).isInstanceOf(BlockingOperationError.class) for instance), then they'll need to explicitly add the testCompile dependency to blockhound.
There is a bigger problem - integrations and customization. Without blockhound
on compile
classpath, the users would struggle to understand how to apply customizations
There is a bigger problem - integrations and customization. Without
blockhound
oncompile
classpath, the users would struggle to understand how to apply customizations
yeah that's a fair point, having it as compile helps with discoverability of customizations. it might be that the default integrations are enough for a lot of users, but still.
ok, so your arguments shift the weight towards a fix, which is:
- make
blockhound-junit-platform
use thejava-library
plugin - have it declare an
api
dependency onblockhound
This should suffice in reverting the difference in the produced pom.xml
.
I think I can provide that fix tomorrow, would you be able to double check everything by the end of the week? In which case I could do a fixup release on Monday. AFAIK there are no features lined up so it doesn't make much sense to wait more than that, unless you have anything in mind @bsideup?
@simonbasle thanks! I think we're good releasing it ASAP and not wait for more changes 👍