> if any of the non XalanJ persons create any of the PRs that XalanJ
> committers might review.

Do you think you have cycles to review
https://github.com/apache/xalan-java/pull/1 <-- This is a trivial change,
and impacts users
and
https://github.com/apache/xalan-java/pull/2
?

>if any of the non XalanJ persons create any of the PRs that XalanJ
>committers might review.

I am not comfortable creating PRs when there's no CI to capture bugs.
I expect that enabling tests in CI would attract contributors, and it would
allow capturing bugs earlier.

>But right now, no one other than *myself* is trying to
>commit any dev code changes within XalanJ repos

I see it might sound awkward to create PRs "for your own",
however, I believe creating PRs would invite others to review:
* PR gives a better understanding of what has changed
* they could see the code compiles and it passes tests
* they could comment on the lines of code
* PR can contain a work-in-progress state for early preview

Have you considered inviting me as a committer?

I'm not using Xalan extensively, however, I'm maintaining Apache JMeter,
and it relies on Xalan in public APIs.

> But I'd wish, if someone may spend enough time to study the logic
> within the .xsl files that're provided with this test suite, and point
> out the defects, instead of *just/only* looking at method names,

I can't speak for everybody, however, it is unsatisfying to review code
that uses obscure variables and method names.
It is hard to review code that takes bits from different places rather than
pass string literals.

Have you considered using string literals for the input data?
Let us discuss a test:
https://github.com/apache/xalan-java/blob/986ec08c365a57853d09f7ffaa99238528e853ee/tests/org/apache/xalan/xpath3/FnTokenizeTests.java#L78-L82

input: test1_a.xml (2 lines)
xls: test1.xsl (38 lines)
output: test1.out (4 lines)

In fact, the input XML is just <?xml ...?><elem>abracadabra</elem>
XLS contains two tests:
   <xsl:template match="/elem">
      <elem>
        <count><xsl:value-of select="count(.)"/></count>
        <count><xsl:value-of select="count(tokenize(.,
'(ab)|(a)'))"/></count>
      </elem>
   </xsl:template>

Frankly speaking, I have no idea what count(.) is doing there. Is it
testing fn-tokenize somehow?
It does not look connected with the fn-tokenize.
So I believe it is not really needed for the test.

It looks like the actual test behind FnTokenizeTests.test1 is as follows:

given input "abracadabra", and template "count(tokenize(., '(ab)|(a)'))"
expect "6" as the output.

That one line describes 2+38+4=40 lines of the test data in the repository.

Now as the meaning of the test is reverse-engineered, let us try to make it
easier to understand for everybody:

class FnTokenizeTest {
    @Test
    public void countTokenizeRegexp() {
        assertValueOf(/* input = */ "abracadabra", /* xls = */
"count(tokenize(., '(ab)|(a)'))", /* output = */ "6");
    }
}

The assertValueOf method could create in-memory XMLs for input and XSL, and
it would likely be useful for many other tests as well.
Modern Java and Kotlin have even multiline string literals, so declaring
XMLs in place would make it much easier to create, and review tests.

I hope that helps understand my suggestion.

Vladimir

Reply via email to