kriegaex commented on code in PR #129:
URL: https://github.com/apache/xalan-java/pull/129#discussion_r1407301019
##########
serializer/src/main/java/org/apache/xml/serializer/Version.java:
##########
@@ -55,7 +55,16 @@ public final class Version
private static void readProperties() {
Properties pomProperties = new Properties();
- try (InputStream fromResource =
Version.class.getClassLoader().getResourceAsStream(POM_PROPERTIES_PATH)) {
+ ClassLoader classLoader = Version.class.getClassLoader();
+ if (classLoader == null) {
+ // Oops! Someone put Xalan is on the bootstrap class loader (BCL) -> fall
+ // back to the system class loader, because there is no Classloader
+ // instance for the BCL (native code). Due to class loader hierarchy,
+ // however, the resource will also be found when asking for it from a
+ // level below the BCL.
+ classLoader = ClassLoader.getSystemClassLoader();
+ }
+ try (InputStream fromResource =
classLoader.getResourceAsStream(POM_PROPERTIES_PATH)) {
Review Comment:
First of all, @vlsi, I am glad to take suggestions from you, but no orders.
Earlier we talked about test automation several times, and nowhere did anyone
tell me to add test plugins or dependencies before the Maven cutover. @jkesselm
added a JUnit 4 dependency for future use, but told me it was out of scope for
the cutover. Now you are sort of commanding me to do the opposite, i.e. use
JUnit 5 (Jupiter) instead of 4 and add tests instead of waiting for the
cutover. Whom should I follow now? Are you in any position to speak
authoritatively and command me to do such things? You are not even working
actively on the Maven branch, because in your opinion Maven is a (quote) "waste
of time".
Theoretically assuming for a minute, I would add tests separately from
xalan-test, what would be the consequence? Tests would diverge, because they
would be maintained in two places. After the cutover, some of the xalan-test
tests might need adjustment anyway, if they make any assumptions about project
structure, dependencies etc. As soon as anyone would start merging them into
this project to make things easier in the future, that person would have to
reconcile my tests with the migrated ones. We might even have two different
test frameworks, JUnit 4/5 vs. the currently used home-brew framework of
xalan-test. I guess, in a first step when migrating them into this project, we
would need to run them from Surefire (unit tests) and Failsafe + Invoker
(integration tests) as-is, i.e. make sure to run them without first migrating
them to JUnit, if the latter would be a huge effort. My handful of new tests
might even be an impediment during that phase. I can easily add those test
plugins
and write a handful of tests, but it is not my decision to start adding big
new features to the Maven build, further stalling the cutover.
Besides, I did not add the version modules. They already existed without any
test coverage. I merely ported them to the Maven build and according to the
boyscout rule changed the way they work while retaining the API, because I
wanted to avoid code generation magic in favour of simple resource filtering.
So, thanks for your opinion. I know you want the right thing. I am not
arguing about that, we are on the same page there. I simply do not want to mix
this with the Maven cutover, so our disagreement is not about "new tests
yes/no" but about "new tests now/later".
@jkesselm, I know we are talking post factum, because the PR was already
merged. Would you have me add test frameworks and tests rather now or later? If
now, then which testing tool and version are you envisioning? My suggestion is
Spock, an advances testing framework running as a separate engine on top of
JUnit 5 platform, i.e. as a sibling of Jupiter. A more conservative person
would probably suggest JUnit 5 Jupiter, an even more conservative one JUnit 4.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]