You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@xalan.apache.org by "jkesselm (via GitHub)" <gi...@apache.org> on 2023/04/27 02:49:02 UTC
[GitHub] [xalan-test] jkesselm opened a new pull request, #1: Move tests/2.7.3 into main test driver framework
jkesselm opened a new pull request, #1:
URL: https://github.com/apache/xalan-test/pull/1
- Created tests/jira directory, implemented drivers and build targets replicating support for mixed stylesheet and java testlets as used in tests/bugzilla.
- Rewrote tests/2.7.3/* into tests/jira java testlets, permitting results to be validated more directly and specifically,
- Removed now-unneeded tests/2.7.3 directory.
- Added a few words of documentation in various places.
Changes glanced at but not made:
- Arguably all the build targets which require .classes builds should have those as dependencies
- I'm not sure I added deleting the tests/jira/build directory to the clean target; should check that.
- *Could* check in Eclipse .classpath and .project files, for convenience. That may want to wait until after moving xalan-test into xalan-java.
- Ideally, any tests in the bugzilla or jira buckets which we now pass should be moved into the appropriate group of functional tests, so we know they're resolved but continue checking for regression. But that would require a bit more reconciling of the current distinction between directories containing java testlets, directories containing .xsl testlets, and the special cases for bugzilla and jira reports.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
Re: [GitHub] [xalan-test] elharo commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by Gary Gregory <ga...@gmail.com>.
I agree on the line length issue. I'm not a fan of "newspaper column"
formatting aside for some fluent API examples. I use a line length of 160
in most projects.
Gary
On Tue, May 9, 2023, 14:23 elharo (via GitHub) <gi...@apache.org> wrote:
>
> elharo commented on code in PR #1:
> URL: https://github.com/apache/xalan-test/pull/1#discussion_r1188967509
>
>
> ##########
> tests/bugzilla/Bugzilla1251.java:
> ##########
> @@ -37,6 +37,19 @@
> * Testlet for reproducing Bugzilla reported bugs.
> *
> * Reported-by: slobo@matavnet.hu
> + *
> + * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user,
> identity transformer reports FAIL (throws exception),
> + * real transformer reports PASS (exception caught and handled as
> intended).
> + *
> + * In real transformer, ElemCopy catches the SaxException, throws it
> wrapped in TransformerException;
> + * TransformerImpl catches that, sees that it has an
> m_serializationHandler. and passes it off to
> + * that. Since that reports checkPass, test is considered good.
> + *
> + * In TransformerIdentityImpl.startElement(), there is no equivalent
> catch-and-handle for exceptions.
> + *
> + * RECOMMENDATION: SANITY CHECK. User appears to have misstated the
> problem. If so -- if the description is
>
> Review Comment:
> -- -> —
>
>
>
> ##########
> tests/bugzilla/Bugzilla1110.java:
> ##########
> @@ -31,6 +31,12 @@
> import org.w3c.dom.traversal.NodeIterator;
> import org.xml.sax.InputSource;
>
> +// jkesselm May 2023: CAN NOT REPRODUCE; prints what the user says they
> expected to see.
>
> Review Comment:
> nit: CANNOT
>
>
>
> ##########
> tests/bugzilla/Bugzilla1266.java:
> ##########
> @@ -74,52 +94,58 @@ public void execute(Datalet d)
> Templates templates = null;
> Transformer transformer = null;
> try
> - {
> - factory = TransformerFactory.newInstance();
> - logger.logMsg(Logger.STATUSMSG, "About to
> factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
> - templates = factory.newTemplates(new
> StreamSource(QetestUtils.filenameToURL("identity.xsl")));
> - transformer = templates.newTransformer();
> -
> - // Set the errorListener and validate it
> - transformer.setErrorListener(loggingErrorListener);
> - if (transformer.getErrorListener() == loggingErrorListener)
> - logger.checkPass("set/getErrorListener on transformer");
> - else
> - logger.checkFail("set/getErrorListener on transformer");
> -
> - logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 -
> warning due to bad output props not propagated");
> - logger.logMsg(Logger.STATUSMSG,
> "transformer.setOutputProperty(encoding, illegal-encoding-value)");
> - transformer.setOutputProperty("encoding",
> "illegal-encoding-value");
> -
> - logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
> - transformer.transform(new
> StreamSource(QetestUtils.filenameToURL("identity.xml")),
> - new StreamResult("Bugzilla1266.out"));
> - logger.logMsg(Logger.STATUSMSG, "after transform(...)");
> - logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after
> transform:" + loggingErrorListener.getQuickCounters());
> -
> - // Validate that one warning (about illegal-encoding-value)
> should have been reported
> - int[] errCtr = loggingErrorListener.getCounters();
> - if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
> - logger.checkPass("At least one Warning listned to for
> illegal-encoding-value");
> - else
> - logger.checkFail("At least one Warning listned to for
> illegal-encoding-value");
> + {
> + factory = TransformerFactory.newInstance();
> + logger.logMsg(Logger.STATUSMSG, "About to
> factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
> + templates = factory.newTemplates(new
> StreamSource(QetestUtils.filenameToURL("identity.xsl")));
> + transformer = templates.newTransformer();
> +
> + // Set the errorListener and validate it
> + transformer.setErrorListener(loggingErrorListener);
> + if (transformer.getErrorListener() ==
> loggingErrorListener)
> + logger.checkPass("set/getErrorListener on
> transformer");
> + else {
> + logger.checkFail("set/getErrorListener on
> transformer");
> + throw new Exception("failed set/getErrorListener on
> transformer");
> + }
> +
> + logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 -
> warning due to bad output props not propagated");
> + logger.logMsg(Logger.STATUSMSG,
> "transformer.setOutputProperty(encoding, illegal-encoding-value)");
> + transformer.setOutputProperty("encoding",
> "illegal-encoding-value");
> +
> + logger.logMsg(Logger.STATUSMSG, "about to
> transform(...)");
> + transformer.transform(new
> StreamSource(QetestUtils.filenameToURL("identity.xml")),
> + new
> StreamResult("Bugzilla1266.out"));
> + logger.logMsg(Logger.STATUSMSG, "after transform(...)");
> + logger.logMsg(Logger.STATUSMSG, "loggingErrorListener
> after transform:" + loggingErrorListener.getQuickCounters());
> +
> + // Validate that one warning (about
> illegal-encoding-value) should have been reported
> + int[] errCtr = loggingErrorListener.getCounters();
> +
> + if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
> + logger.checkPass("At least one Warning listned to for
> illegal-encoding-value");
> + else {
> + logger.checkFail("No Warning reported by listener for
> illegal-encoding-value");
> + throw new Exception("No Warning reported by listener
> for illegal-encoding-value");
> + }
>
> - // Validate the actual output file as well: in this case,
> - // the stylesheet should still work
> - CheckService fileChecker = new XHTFileCheckService();
> - fileChecker.check(logger,
> - new File("Bugzilla1266.out"),
> - new File("identity.gold"),
> - "transform of good xsl w/bad output props into: " +
> "Bugzilla1266.out");
> + // Validate the actual output file as well: in this case,
> + // the stylesheet should still work
> + CheckService fileChecker = new XHTFileCheckService();
> + fileChecker.check(logger,
> + new File("Bugzilla1266.out"),
>
> Review Comment:
> This indent seems excessive
>
>
>
> --
> 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: dev-unsubscribe@xalan.apache.org
>
> For queries about this service, please contact Infrastructure at:
> users@infra.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
> For additional commands, e-mail: dev-help@xalan.apache.org
>
>
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189162388
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
Sorry, I write in 8-bit ASCII unless there is good reason not to. Suggestion declined.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] elharo commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189223321
##########
tests/bugzilla/Bugzilla1266.java:
##########
@@ -74,52 +94,58 @@ public void execute(Datalet d)
Templates templates = null;
Transformer transformer = null;
try
- {
- factory = TransformerFactory.newInstance();
- logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
- templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
- transformer = templates.newTransformer();
-
- // Set the errorListener and validate it
- transformer.setErrorListener(loggingErrorListener);
- if (transformer.getErrorListener() == loggingErrorListener)
- logger.checkPass("set/getErrorListener on transformer");
- else
- logger.checkFail("set/getErrorListener on transformer");
-
- logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
- logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
- transformer.setOutputProperty("encoding", "illegal-encoding-value");
-
- logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
- transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
- new StreamResult("Bugzilla1266.out"));
- logger.logMsg(Logger.STATUSMSG, "after transform(...)");
- logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
-
- // Validate that one warning (about illegal-encoding-value) should have been reported
- int[] errCtr = loggingErrorListener.getCounters();
- if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
- logger.checkPass("At least one Warning listned to for illegal-encoding-value");
- else
- logger.checkFail("At least one Warning listned to for illegal-encoding-value");
+ {
+ factory = TransformerFactory.newInstance();
+ logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
+ templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
+ transformer = templates.newTransformer();
+
+ // Set the errorListener and validate it
+ transformer.setErrorListener(loggingErrorListener);
+ if (transformer.getErrorListener() == loggingErrorListener)
+ logger.checkPass("set/getErrorListener on transformer");
+ else {
+ logger.checkFail("set/getErrorListener on transformer");
+ throw new Exception("failed set/getErrorListener on transformer");
+ }
+
+ logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
+ logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
+ transformer.setOutputProperty("encoding", "illegal-encoding-value");
+
+ logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
+ transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
+ new StreamResult("Bugzilla1266.out"));
+ logger.logMsg(Logger.STATUSMSG, "after transform(...)");
+ logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
+
+ // Validate that one warning (about illegal-encoding-value) should have been reported
+ int[] errCtr = loggingErrorListener.getCounters();
+
+ if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
+ logger.checkPass("At least one Warning listned to for illegal-encoding-value");
+ else {
+ logger.checkFail("No Warning reported by listener for illegal-encoding-value");
+ throw new Exception("No Warning reported by listener for illegal-encoding-value");
+ }
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
Review Comment:
Whether it's new or not I see a 17 space indent here, which is way too much. I think I see why: it's trying to line up the arguments. If spotless is doing this, then that's another reason to hate on spotless, but for now not a blocker. If it's your editor please fix that before committing.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] garydgregory commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1544414243
Anyone who is a committer should be able to merge directly from GitHub.
Gary
On Thu, May 11, 2023, 13:30 Joe Kesselman ***@***.***> wrote:
> Thanks, Gary. Could you clarify who has approval/merge capability at this
> point? I know we're still shorthanded so I don't want to pester folks, but
> it'd help level-set me.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/xalan-test/pull/1#issuecomment-1544405304>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJB6N3SN3GPZFC7FMUM5BTXFUOZXANCNFSM6AAAAAAXNHL2SY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1546730860
Thank ye kindly, sir. I'll look at fixing the two reproduced issues.
Worth discussing what we do with the other tests, or should I just submit a proposal?
--
/_ Joe Kesselman (he/him/his)
-/ _) My Alexa skill for New Music/New Sounds fans:
/ https://www.amazon.com/dp/B09WJ3H657/
() Plaintext Ribbon Campaign
/\ Stamp out HTML mail!
________________________________
From: Gary Gregory ***@***.***>
Sent: Saturday, May 13, 2023 12:46:12 PM
To: apache/xalan-test ***@***.***>
Cc: Joe Kesselman ***@***.***>; Mention ***@***.***>
Subject: Re: [apache/xalan-test] Move tests/2.7.3 into main test driver framework, document bugzilla test status (PR #1)
Merged #1<https://github.com/apache/xalan-test/pull/1> into master.
—
Reply to this email directly, view it on GitHub<https://github.com/apache/xalan-test/pull/1#event-9238924835>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A7OJ6W3PIJX6F3IYYC3LLWTXF63FJANCNFSM6AAAAAAXNHL2SY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542587344
Speaking of nitpicking, I now think I'm *not* accepting the em-dash request as a "friendly amendment". Coding is not typesetting. I'll accept the preference for em-space in javadoc or other text intended primarily for publication, but in a code comment it's really not necessary, or (I would argue) justified. It's a nitpick at best, and potentially a barrier to file searches at the worst. If someone else wants to use em-dash characters in comment text, I am willing to let them do so, but I don't think I consider it an improvement and am backing it out of my submission. If you want to make it a stylistic requirement, we first need to get consensus on an explicit set of style rules and, preferably, automate their application. Again, don't create contribution barriers where they aren't necessary.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1544405304
Thanks, Gary. Could you clarify who has approval/merge capability at this point? I know we're still shorthanded so I don't want to pester folks, but it'd help level-set me.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1546700059
Squash, by all means.
Valid point re tooling configs and code style.
--
/_ Joe Kesselman (he/him/his)
-/ _) My Alexa skill for New Music/New Sounds fans:
/ https://www.amazon.com/dp/B09WJ3H657/
() Plaintext Ribbon Campaign
/\ Stamp out HTML mail!
________________________________
From: Gary Gregory ***@***.***>
Sent: Saturday, May 13, 2023 11:30:43 AM
To: apache/xalan-test ***@***.***>
Cc: Joe Kesselman ***@***.***>; Mention ***@***.***>
Subject: Re: [apache/xalan-test] Move tests/2.7.3 into main test driver framework, document bugzilla test status (PR #1)
@garydgregory commented on this pull request.
@jkesselm<https://github.com/jkesselm> and all:
Do you care about the commit history or can I squash & commit? I generally squash PRs assuming that the commits are WIP (unless I hear otherwise).
I would prefer we NOT store Eclipse, IntelliJ, and assorted editor configuration files. It is too painful to sync; instead we should, IMO: Migrate to Maven and enforce style with the Maven Checkstyle plugin or use the Spotless plugin in addition to PMD and SpotBugs.
—
Reply to this email directly, view it on GitHub<https://github.com/apache/xalan-test/pull/1#pullrequestreview-1425426992>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A7OJ6WZTEOSNHAUFJNK7KODXF6SKHANCNFSM6AAAAAAXNHL2SY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189162388
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
Sorry, I write in 8-bit ASCII unless there is good reason not to. Suggestion declined unless there is a W3C style guide mandating otherwise.
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
Sorry, I write in 7-bit ASCII unless there is good reason not to. Suggestion declined unless there is a W3C style guide mandating otherwise.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] garydgregory commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1540965355
> (Switching from ant build to maven build is something that has been suggested but that we don't yet have clear consensus on, and would of course be a separate effort.)
I'm all for migratin to Maven, here is a ticket https://issues.apache.org/jira/browse/XALANJ-2651
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1544494343
Hm. Git tells me that the outstanding requested review is not required before merging (though I'd rather wait until you've said OK), but it is not showing me the merge/squash-and-merge button I'd expect to see if I could proceed. I'm not sure whether I'm missing something obvious as a user, or missing a bit in my permissions.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] garydgregory commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1543835809
I'm about to travel today, I should be able to review over the weekend
unless someone gets to it first.
Gary
On Thu, May 11, 2023, 07:30 mukulga ***@***.***> wrote:
> I'd like to create a branch xalan-j_xslt3.0 from xalan-test repos's master
> branch (to help test xalan-java repos branch xalan-j_xslt3.0 codebase),
> before this PR is merged to xalan-test repos's master branch.
>
> Later (i.e, after this PR is merged), we can merge changes from xalan-test
> repos's master branch, to xalan-test repos's branch xalan-j_xslt3.0.
>
> I've created the branch xalan-j_xslt3.0 from xalan-test repos's master
> branch.
>
> I'm fine, if this Joseph's PR is merged to the asked xalan-test
> destination branch. Perhaps, @garydgregory
> <https://github.com/garydgregory> could review this PR and do this
> codebase merge.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/xalan-test/pull/1#issuecomment-1543827022>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJB6N6RVDJQLBTIL2NJ5M3XFTEU7ANCNFSM6AAAAAAXNHL2SY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542580670
1. Historically, in this project, we have NOT used eight-space indentations; I believe we started with four. When in doubt, follow project's existing conventions. And your comment about line length directly conflicts with this request.
2. My auto-formatters (Emacs, Eclipse) happily line up arguments. Most others I've used likewise; it's just a matter of setting the style parameter appropriately.
3. What you're calling "badly formatted code" is established code style for that file/project. If you consider the existing codebase as a whole badly formatted, see your own advice about general reformats. There are many formatting styles, and Oracle's is far from universally accepted. If you want to switch to that, it can be done, but see your own advice about applying that globally as a separate PR -- and if this is a priority for you, seriously consider making the rules being followed part of the codebase and make code reflow part of the commit process.
This is not a hill I want to die on, but it's also simply not as important to me as functionality and human-readability. It's also a gratuitous barrier to participation -- and we've recognized that participation is a serious problem for us right now. Nitpicking is fine and appreciated, but should not be a barrier to progress.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189171486
##########
tests/bugzilla/Bugzilla1266.java:
##########
@@ -74,52 +94,58 @@ public void execute(Datalet d)
Templates templates = null;
Transformer transformer = null;
try
- {
- factory = TransformerFactory.newInstance();
- logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
- templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
- transformer = templates.newTransformer();
-
- // Set the errorListener and validate it
- transformer.setErrorListener(loggingErrorListener);
- if (transformer.getErrorListener() == loggingErrorListener)
- logger.checkPass("set/getErrorListener on transformer");
- else
- logger.checkFail("set/getErrorListener on transformer");
-
- logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
- logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
- transformer.setOutputProperty("encoding", "illegal-encoding-value");
-
- logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
- transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
- new StreamResult("Bugzilla1266.out"));
- logger.logMsg(Logger.STATUSMSG, "after transform(...)");
- logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
-
- // Validate that one warning (about illegal-encoding-value) should have been reported
- int[] errCtr = loggingErrorListener.getCounters();
- if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
- logger.checkPass("At least one Warning listned to for illegal-encoding-value");
- else
- logger.checkFail("At least one Warning listned to for illegal-encoding-value");
+ {
+ factory = TransformerFactory.newInstance();
+ logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
+ templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
+ transformer = templates.newTransformer();
+
+ // Set the errorListener and validate it
+ transformer.setErrorListener(loggingErrorListener);
+ if (transformer.getErrorListener() == loggingErrorListener)
+ logger.checkPass("set/getErrorListener on transformer");
+ else {
+ logger.checkFail("set/getErrorListener on transformer");
+ throw new Exception("failed set/getErrorListener on transformer");
+ }
+
+ logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
+ logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
+ transformer.setOutputProperty("encoding", "illegal-encoding-value");
+
+ logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
+ transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
+ new StreamResult("Bugzilla1266.out"));
+ logger.logMsg(Logger.STATUSMSG, "after transform(...)");
+ logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
+
+ // Validate that one warning (about illegal-encoding-value) should have been reported
+ int[] errCtr = loggingErrorListener.getCounters();
+
+ if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
+ logger.checkPass("At least one Warning listned to for illegal-encoding-value");
+ else {
+ logger.checkFail("No Warning reported by listener for illegal-encoding-value");
+ throw new Exception("No Warning reported by listener for illegal-encoding-value");
+ }
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
Review Comment:
One common indentation style for multi-line parameters, as seen earlier in the transformer.transform() call, is to align the parameters. As seen in the web view of this comment, that is in fact what's happening. Indentation has been done with spaces rather than tabs (safer when some editors allow setting the tab stops), but the style is consistent; I'm not sure why you're flagging this one. Clarify, please?
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1540960141
(Switching from ant build to maven build is something that has been suggested but that we don't yet have clear consensus on, and would of course be a separate effort.)
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] garydgregory merged pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #1:
URL: https://github.com/apache/xalan-test/pull/1
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542593330
So, recognizing that the code-style comments are nitpicks and that code style should be separated from functional change, can we get consensus that this change set can be merged? If not, how can we drive toward that consensus?
I now have committer authority. I do not, apparently, have merge authority. Who does, and what do they need to see from us before acceptance?
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542682359
We (meaning the IBM group) used to have a shared Eclipse profile that included some code style settings, which helped ensure some consistency. Apparently that didn't get checked into the project, probably because we were using a different source manager (subversion?) which didn't encourage that.
We could derive the existing conventions from the code, assuming that the majority of the codebase hasn't changed much since then. (Which, alas, is probably true.) Or we can try to get consensus and push it into that form, confirming that the change results in the same byte codes (modulo debugging line numbers) as a rigorous proof that the change has no side effects.
This is near the bottom of my own priority list; I'm a firm believer that code style consistency is a tool toward code readability and that clarity of intent trumps formalism. I'm certainly not going to deliberately break it, but I'm not very motivated to invest my own time on changing what already exists.
(I'm also a believer that a competent programmer should be able to both read and write most of the existing style conventions, though I recognize that Apache code is partly pedagogical. And I only say "most" because I've been exposed to some hideous examples, like the guy who thought semicolons belonged at the start of C statements rather than the end.)
--
/_ Joe Kesselman (he/him/his)
-/ _) My Alexa skill for New Music/New Sounds fans:
/ https://www.amazon.com/dp/B09WJ3H657/
() Plaintext Ribbon Campaign
/\ Stamp out HTML mail!
________________________________
From: Elliotte Rusty Harold ***@***.***>
Sent: Wednesday, May 10, 2023 2:29:53 PM
To: apache/xalan-test ***@***.***>
Cc: Joe Kesselman ***@***.***>; Author ***@***.***>
Subject: Re: [apache/xalan-test] Move tests/2.7.3 into main test driver framework, document bugzilla test status (PR #1)
I think we fundamentally agree. I'm fine with four space indents on continuation lines, or whatever project conventions folks like, provided it's written down somewhere. Are there code conventions for this project? I looked but couldn't find any. Ideally we would also have autoformatters on submit (not format checks but actual formatters running on the source repo) that make formatting nits moot. Absent explicit style guidelines, formatting tends to vary idiosyncratcially from file to file and line to line, and there's a lot of confusing churn as different editors autoformat to different conventions.
—
Reply to this email directly, view it on GitHub<https://github.com/apache/xalan-test/pull/1#issuecomment-1542631630>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A7OJ6W4C6VUGCFHX3J6F5VDXFPNCDANCNFSM6AAAAAAXNHL2SY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] mukulga commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "mukulga (via GitHub)" <gi...@apache.org>.
mukulga commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1546514081
> Before deciding what to do with each of these, we should really make sure that running it on at least one other platform (eg Windows) produces the same results. But as far as Linux is concerned, the two which are confirmed to be genuine glitches are 1251 and 1266, and I have documented the apparent causes and proposed fixes for both.
>
> That gets rid of a lot of suspected technical debt!
>
> Once we have consensus on these, it would be nice to burn down the Jira backlog too.
>
> That would leave us with a clean (as far as we know) implementation of Xalan 2, which would be better for the users and a more stable base for further work.
I agree with you. I'm personally interested to, try XalanJ implementation testing (on windows perhaps to start with) with the improvements provided by this PR. I think, we should merge this PR to the asked xalan-test destination branch (i.e, 'master'). Then we could take a pull of new contents (which shall have improvements provided by this PR) of xalan-test 'master' branch, and do local testing of XalanJ implementation with tests provided by xalan-test repo's branch 'master'.
I hope that, @garydgregory shall find time at his earliest convenience, to merge this PR.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189171486
##########
tests/bugzilla/Bugzilla1266.java:
##########
@@ -74,52 +94,58 @@ public void execute(Datalet d)
Templates templates = null;
Transformer transformer = null;
try
- {
- factory = TransformerFactory.newInstance();
- logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
- templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
- transformer = templates.newTransformer();
-
- // Set the errorListener and validate it
- transformer.setErrorListener(loggingErrorListener);
- if (transformer.getErrorListener() == loggingErrorListener)
- logger.checkPass("set/getErrorListener on transformer");
- else
- logger.checkFail("set/getErrorListener on transformer");
-
- logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
- logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
- transformer.setOutputProperty("encoding", "illegal-encoding-value");
-
- logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
- transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
- new StreamResult("Bugzilla1266.out"));
- logger.logMsg(Logger.STATUSMSG, "after transform(...)");
- logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
-
- // Validate that one warning (about illegal-encoding-value) should have been reported
- int[] errCtr = loggingErrorListener.getCounters();
- if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
- logger.checkPass("At least one Warning listned to for illegal-encoding-value");
- else
- logger.checkFail("At least one Warning listned to for illegal-encoding-value");
+ {
+ factory = TransformerFactory.newInstance();
+ logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
+ templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
+ transformer = templates.newTransformer();
+
+ // Set the errorListener and validate it
+ transformer.setErrorListener(loggingErrorListener);
+ if (transformer.getErrorListener() == loggingErrorListener)
+ logger.checkPass("set/getErrorListener on transformer");
+ else {
+ logger.checkFail("set/getErrorListener on transformer");
+ throw new Exception("failed set/getErrorListener on transformer");
+ }
+
+ logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
+ logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
+ transformer.setOutputProperty("encoding", "illegal-encoding-value");
+
+ logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
+ transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
+ new StreamResult("Bugzilla1266.out"));
+ logger.logMsg(Logger.STATUSMSG, "after transform(...)");
+ logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
+
+ // Validate that one warning (about illegal-encoding-value) should have been reported
+ int[] errCtr = loggingErrorListener.getCounters();
+
+ if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
+ logger.checkPass("At least one Warning listned to for illegal-encoding-value");
+ else {
+ logger.checkFail("No Warning reported by listener for illegal-encoding-value");
+ throw new Exception("No Warning reported by listener for illegal-encoding-value");
+ }
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
Review Comment:
One common indentation style for multi-line parameters, as seen earlier in the transformer.transform() call, is to align the parameters. As seen in the web view of this comment, that is in fact what's happening. Indentation has been done with spaces rather than tabs (safer when some editors allow setting the tab stops), but the style is consistent; I'm not sure why you're flagging this one. Clarify, please?
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] elharo commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542639314
I'm OK with this being merged though formatting guidelines are one of many things we should deal with sooner rather than later, and certainly in advance of any significant feature work. I approved this a while back. I don't know who can merge this or under what conditions.
Which reminds me, another decision we should be explicit about is merges and commit history. I am very strongly in favor of squash merges only, and configuring the Github repo to mandate that. One PR == one git commit.
I even more strongly want to avoid any particular rules about formatting of git commit messages. They're a huge amount of extra hassle for no particular benefit in my experience.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1546500168
Before deciding what to do with each of these, we should really make sure that running it on at least one other platform (eg Windows) produces the same results. But as far as Linux is concerned, the two which are confirmed to be genuine glitches are 1251 and 1266, and I have documented the apparent causes and proposed fixes for both.
That gets rid of a lot of suspected technical debt!
Once we have consensus on these, it would be nice to burn down the Jira backlog too.
That would leave us with a clean (as far as we know) implementation of Xalan 2, which would be better for the users and a more stable base for further work.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189227238
##########
tests/bugzilla/Bugzilla1266.java:
##########
@@ -74,52 +94,58 @@ public void execute(Datalet d)
Templates templates = null;
Transformer transformer = null;
try
- {
- factory = TransformerFactory.newInstance();
- logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
- templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
- transformer = templates.newTransformer();
-
- // Set the errorListener and validate it
- transformer.setErrorListener(loggingErrorListener);
- if (transformer.getErrorListener() == loggingErrorListener)
- logger.checkPass("set/getErrorListener on transformer");
- else
- logger.checkFail("set/getErrorListener on transformer");
-
- logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
- logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
- transformer.setOutputProperty("encoding", "illegal-encoding-value");
-
- logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
- transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
- new StreamResult("Bugzilla1266.out"));
- logger.logMsg(Logger.STATUSMSG, "after transform(...)");
- logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
-
- // Validate that one warning (about illegal-encoding-value) should have been reported
- int[] errCtr = loggingErrorListener.getCounters();
- if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
- logger.checkPass("At least one Warning listned to for illegal-encoding-value");
- else
- logger.checkFail("At least one Warning listned to for illegal-encoding-value");
+ {
+ factory = TransformerFactory.newInstance();
+ logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
+ templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
+ transformer = templates.newTransformer();
+
+ // Set the errorListener and validate it
+ transformer.setErrorListener(loggingErrorListener);
+ if (transformer.getErrorListener() == loggingErrorListener)
+ logger.checkPass("set/getErrorListener on transformer");
+ else {
+ logger.checkFail("set/getErrorListener on transformer");
+ throw new Exception("failed set/getErrorListener on transformer");
+ }
+
+ logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
+ logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
+ transformer.setOutputProperty("encoding", "illegal-encoding-value");
+
+ logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
+ transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
+ new StreamResult("Bugzilla1266.out"));
+ logger.logMsg(Logger.STATUSMSG, "after transform(...)");
+ logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
+
+ // Validate that one warning (about illegal-encoding-value) should have been reported
+ int[] errCtr = loggingErrorListener.getCounters();
+
+ if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
+ logger.checkPass("At least one Warning listned to for illegal-encoding-value");
+ else {
+ logger.checkFail("No Warning reported by listener for illegal-encoding-value");
+ throw new Exception("No Warning reported by listener for illegal-encoding-value");
+ }
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
Review Comment:
Sigh. Code style conventions.
This is exactly the same style (vertical alignment of arguments) used elsewhere in that same .java file. If we're gonna object to it, we should be reformatting the entire testcase to whatever our established preferred style is.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] mukulga commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "mukulga (via GitHub)" <gi...@apache.org>.
mukulga commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1543827022
> I'd like to create a branch xalan-j_xslt3.0 from xalan-test repos's master branch (to help test xalan-java repos branch xalan-j_xslt3.0 codebase), before this PR is merged to xalan-test repos's master branch.
>
> Later (i.e, after this PR is merged), we can merge changes from xalan-test repos's master branch, to xalan-test repos's branch xalan-j_xslt3.0.
I've created the branch xalan-j_xslt3.0 from xalan-test repos's master branch.
I'm fine, if this Joseph's PR is merged to the asked xalan-test destination branch. Perhaps, @garydgregory could review this PR and do this codebase merge.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] elharo commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1540802716
My thinking is this. Is there currently an equivalent of `mvn test` for this repo? E.g. ant test or ./build.sh test or something like that? And if so does that currently pass? If the tests for this repo are currently passing (after whatever other config and setup steps are required) then I'd prefer not to merge PRs that break them.
If, OTOH, this doesn't even build yet and this moves us closer to making it build and tests pass, then we might as go ahead and merge. I'm not familiar with somewhat idiosyncratic Xalan build, so I'm not sure which case is true.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189196871
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
Mumble. I honestly think this is Not An Improvement. We're typing, not typesetting. And -- is pervasive throughout this codebase.
Since folks seem to care, I'm willing to let this instance be changed.
I suppose I could bind a key to M-dash and try to get in the habit of using it in the future.
I am *not* volunteering to do a global grovel through the code and change the 300+ other instances; if it's important to you, feel free to do so and I'll reluctantly agree with the pull requests.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189161889
##########
tests/bugzilla/Bugzilla1110.java:
##########
@@ -31,6 +31,12 @@
import org.w3c.dom.traversal.NodeIterator;
import org.xml.sax.InputSource;
+// jkesselm May 2023: CAN NOT REPRODUCE; prints what the user says they expected to see.
Review Comment:
Nit: Either is correct. Unless there's a W3C style guide which says otherwise, I would rather leave this unchanged.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] mukulga commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "mukulga (via GitHub)" <gi...@apache.org>.
mukulga commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1541477280
I'd like to create a branch xalan-j_xslt3.0 from xalan-test repos's master branch (to help test xalan-java repos branch xalan-j_xslt3.0 codebase), before this PR is merged to xalan-test repos's master branch.
Later (i.e, after this PR is merged), we can merge changes from xalan-test repos's master branch, to xalan-test repos's branch xalan-j_xslt3.0.
Please advise.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] elharo commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542631630
I think we fundamentally agree. I'm fine with four space indents on continuation lines, or whatever project conventions folks like, provided it's written down somewhere. Are there code conventions for this project? I looked but couldn't find any. Ideally we would also have autoformatters on submit (not format checks but actual formatters running on the source repo) that make formatting nits moot. Absent explicit style guidelines, formatting tends to vary idiosyncratcially from file to file and line to line, and there's a lot of confusing churn as different editors autoformat to different conventions.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] elharo commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1188967509
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
-- -> —
##########
tests/bugzilla/Bugzilla1110.java:
##########
@@ -31,6 +31,12 @@
import org.w3c.dom.traversal.NodeIterator;
import org.xml.sax.InputSource;
+// jkesselm May 2023: CAN NOT REPRODUCE; prints what the user says they expected to see.
Review Comment:
nit: CANNOT
##########
tests/bugzilla/Bugzilla1266.java:
##########
@@ -74,52 +94,58 @@ public void execute(Datalet d)
Templates templates = null;
Transformer transformer = null;
try
- {
- factory = TransformerFactory.newInstance();
- logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
- templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
- transformer = templates.newTransformer();
-
- // Set the errorListener and validate it
- transformer.setErrorListener(loggingErrorListener);
- if (transformer.getErrorListener() == loggingErrorListener)
- logger.checkPass("set/getErrorListener on transformer");
- else
- logger.checkFail("set/getErrorListener on transformer");
-
- logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
- logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
- transformer.setOutputProperty("encoding", "illegal-encoding-value");
-
- logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
- transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
- new StreamResult("Bugzilla1266.out"));
- logger.logMsg(Logger.STATUSMSG, "after transform(...)");
- logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
-
- // Validate that one warning (about illegal-encoding-value) should have been reported
- int[] errCtr = loggingErrorListener.getCounters();
- if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
- logger.checkPass("At least one Warning listned to for illegal-encoding-value");
- else
- logger.checkFail("At least one Warning listned to for illegal-encoding-value");
+ {
+ factory = TransformerFactory.newInstance();
+ logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
+ templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
+ transformer = templates.newTransformer();
+
+ // Set the errorListener and validate it
+ transformer.setErrorListener(loggingErrorListener);
+ if (transformer.getErrorListener() == loggingErrorListener)
+ logger.checkPass("set/getErrorListener on transformer");
+ else {
+ logger.checkFail("set/getErrorListener on transformer");
+ throw new Exception("failed set/getErrorListener on transformer");
+ }
+
+ logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
+ logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
+ transformer.setOutputProperty("encoding", "illegal-encoding-value");
+
+ logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
+ transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
+ new StreamResult("Bugzilla1266.out"));
+ logger.logMsg(Logger.STATUSMSG, "after transform(...)");
+ logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
+
+ // Validate that one warning (about illegal-encoding-value) should have been reported
+ int[] errCtr = loggingErrorListener.getCounters();
+
+ if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
+ logger.checkPass("At least one Warning listned to for illegal-encoding-value");
+ else {
+ logger.checkFail("No Warning reported by listener for illegal-encoding-value");
+ throw new Exception("No Warning reported by listener for illegal-encoding-value");
+ }
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
Review Comment:
This indent seems excessive
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] elharo commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1541022900
OK, sounds reasonable. I know things are going to be a bit of a mess as we move to a more standard build system.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1540957611
There is a build.sh in the xalan-tests root directory, with multiple testing targets.
These changes move new tests into the standard test framework by creating a bucket for jira issues, and document the actual status of the old bugzilla bucket. Neither of those buckets is part of standard smoketesting, and in fact tests which pass and which are worth keeping for regression-check purposes should be migrated into other parts of the test tree that _are_ included in smoketest. The old test framework has some limitations which make that migration more difficult than it should be when the test involves a Java driver; fixing that is on my to-do list.
But for now, this makes invocation of the jira-issue tests parallel invocation of the bugzilla-issue tests rather than requiring a "magic" script to launch them outside the test framework, fills in some files that were missing from the bugzilla bucket, and documents the status of the bugzilla tests (obsolete, user error, feature request, or actual bug report). It's a clean-up pass.
- The currently recommended regression tests passed before my changes and continue to pass after them.
- The three new (previously manual) tests for jira issues are now invoked through the test framework.
- The old tests for bugzilla issues have been completed (where files were missing) and status documented.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] garydgregory commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189183183
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
As JEP 400 (https://openjdk.org/jeps/400) confirms, UTF-8 is the way to go. See https://bugs.openjdk.org/browse/JDK-8301971
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1542693920
Absolutely agree about squash merge. Especially since hit can squash as part of the merge. Making each merge self-contained is clearly a win.
And, yeah, let the messages float with just a plea that they be readable descriptions of the intent. I'm willing to accept anything from "doc" on a comment-only change to an essay with citations on why a functional change is needed, with most in the middle somewhere... I just want to be able to look at the history and make a reasonable guess about which change might be involved in any given issue. Including the Jira issue number is always good too, but I'm willing to trust common sense to apply.
--
/_ Joe Kesselman (he/him/his)
-/ _) My Alexa skill for New Music/New Sounds fans:
/ https://www.amazon.com/dp/B09WJ3H657/
() Plaintext Ribbon Campaign
/\ Stamp out HTML mail!
________________________________
From: Elliotte Rusty Harold ***@***.***>
Sent: Wednesday, May 10, 2023 2:36:56 PM
To: apache/xalan-test ***@***.***>
Cc: Joe Kesselman ***@***.***>; Author ***@***.***>
Subject: Re: [apache/xalan-test] Move tests/2.7.3 into main test driver framework, document bugzilla test status (PR #1)
I'm OK with this being merged though formatting guidelines are one of many things we should deal with sooner rather than later, and certainly in advance of any significant feature work. I approved this a while back. I don't know who can merge this or under what conditions.
Which reminds me, another decision we should be explicit about is merges and commit history. I am very strongly in favor of squash merges only, and configuring the Github repo to mandate that. One PR == one git commit.
I even more strongly want to avoid any particular rules about formatting of git commit messages. They're a huge amount of extra hassle for no particular benefit in my experience.
—
Reply to this email directly, view it on GitHub<https://github.com/apache/xalan-test/pull/1#issuecomment-1542639314>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A7OJ6W4CZMUJHFEESDSE5GLXFPN4RANCNFSM6AAAAAAXNHL2SY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on PR #1:
URL: https://github.com/apache/xalan-test/pull/1#issuecomment-1540688850
Thanks for the feedback; I'll take a look at it.
I was thinking to get the files in with the comments first, so we could discuss what to do about them, but I'm certainly willing to act now if preferred.
Re indentation: I expect that's the result of my running the document thru Emacs indent-region with its default settings. I'll take a look.
--
/_ Joe Kesselman (he/him/his)
-/ _) My Alexa skill for New Music/New Sounds fans:
/ https://www.amazon.com/dp/B09WJ3H657/
() Plaintext Ribbon Campaign
/\ Stamp out HTML mail!
________________________________
From: Elliotte Rusty Harold ***@***.***>
Sent: Tuesday, May 9, 2023 2:20:10 PM
To: apache/xalan-test ***@***.***>
Cc: Joe Kesselman ***@***.***>; Author ***@***.***>
Subject: Re: [apache/xalan-test] Move tests/2.7.3 into main test driver framework, document bugzilla test status (PR #1)
@elharo commented on this pull request.
Not carefully reviewing the code since it's just a move. A few nits on new comments. However it does seem there are some tests here that are in a broken state. Should these be removed before merging in the ones that pass?
________________________________
In tests/bugzilla/Bugzilla1110.java<https://github.com/apache/xalan-test/pull/1#discussion_r1188967130>:
> @@ -31,6 +31,12 @@
import org.w3c.dom.traversal.NodeIterator;
import org.xml.sax.InputSource;
+// jkesselm May 2023: CAN NOT REPRODUCE; prints what the user says they expected to see.
nit: CANNOT
________________________________
In tests/bugzilla/Bugzilla1251.java<https://github.com/apache/xalan-test/pull/1#discussion_r1188967509>:
> @@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: ***@***.***
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
-- -> —
________________________________
In tests/bugzilla/Bugzilla1266.java<https://github.com/apache/xalan-test/pull/1#discussion_r1188968165>:
>
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
This indent seems excessive
—
Reply to this email directly, view it on GitHub<https://github.com/apache/xalan-test/pull/1#pullrequestreview-1419201477>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A7OJ6W3OYBGVUC3EOG2IR4DXFKDFVANCNFSM6AAAAAAXNHL2SY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189210403
##########
tests/bugzilla/Bugzilla1266.java:
##########
@@ -74,52 +94,58 @@ public void execute(Datalet d)
Templates templates = null;
Transformer transformer = null;
try
- {
- factory = TransformerFactory.newInstance();
- logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
- templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
- transformer = templates.newTransformer();
-
- // Set the errorListener and validate it
- transformer.setErrorListener(loggingErrorListener);
- if (transformer.getErrorListener() == loggingErrorListener)
- logger.checkPass("set/getErrorListener on transformer");
- else
- logger.checkFail("set/getErrorListener on transformer");
-
- logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
- logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
- transformer.setOutputProperty("encoding", "illegal-encoding-value");
-
- logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
- transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
- new StreamResult("Bugzilla1266.out"));
- logger.logMsg(Logger.STATUSMSG, "after transform(...)");
- logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
-
- // Validate that one warning (about illegal-encoding-value) should have been reported
- int[] errCtr = loggingErrorListener.getCounters();
- if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
- logger.checkPass("At least one Warning listned to for illegal-encoding-value");
- else
- logger.checkFail("At least one Warning listned to for illegal-encoding-value");
+ {
+ factory = TransformerFactory.newInstance();
+ logger.logMsg(Logger.STATUSMSG, "About to factory.newTemplates(" + QetestUtils.filenameToURL("identity.xsl") + ")");
+ templates = factory.newTemplates(new StreamSource(QetestUtils.filenameToURL("identity.xsl")));
+ transformer = templates.newTransformer();
+
+ // Set the errorListener and validate it
+ transformer.setErrorListener(loggingErrorListener);
+ if (transformer.getErrorListener() == loggingErrorListener)
+ logger.checkPass("set/getErrorListener on transformer");
+ else {
+ logger.checkFail("set/getErrorListener on transformer");
+ throw new Exception("failed set/getErrorListener on transformer");
+ }
+
+ logger.logMsg(Logger.STATUSMSG, "Reproduce Bugzilla1266 - warning due to bad output props not propagated");
+ logger.logMsg(Logger.STATUSMSG, "transformer.setOutputProperty(encoding, illegal-encoding-value)");
+ transformer.setOutputProperty("encoding", "illegal-encoding-value");
+
+ logger.logMsg(Logger.STATUSMSG, "about to transform(...)");
+ transformer.transform(new StreamSource(QetestUtils.filenameToURL("identity.xml")),
+ new StreamResult("Bugzilla1266.out"));
+ logger.logMsg(Logger.STATUSMSG, "after transform(...)");
+ logger.logMsg(Logger.STATUSMSG, "loggingErrorListener after transform:" + loggingErrorListener.getQuickCounters());
+
+ // Validate that one warning (about illegal-encoding-value) should have been reported
+ int[] errCtr = loggingErrorListener.getCounters();
+
+ if (errCtr[LoggingErrorListener.TYPE_WARNING] > 0)
+ logger.checkPass("At least one Warning listned to for illegal-encoding-value");
+ else {
+ logger.checkFail("No Warning reported by listener for illegal-encoding-value");
+ throw new Exception("No Warning reported by listener for illegal-encoding-value");
+ }
- // Validate the actual output file as well: in this case,
- // the stylesheet should still work
- CheckService fileChecker = new XHTFileCheckService();
- fileChecker.check(logger,
- new File("Bugzilla1266.out"),
- new File("identity.gold"),
- "transform of good xsl w/bad output props into: " + "Bugzilla1266.out");
+ // Validate the actual output file as well: in this case,
+ // the stylesheet should still work
+ CheckService fileChecker = new XHTFileCheckService();
+ fileChecker.check(logger,
+ new File("Bugzilla1266.out"),
Review Comment:
When I look at this in my editor, indentation appears correct in context (aligned with the '}' above it) with or without untabifying. I'm not at all sure why git is flagging this as an indentation change, unless this was indentation that was previously missing. I'm open to suggestions re how to make git happier with it, but I'm really not seeing any there there.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org
[GitHub] [xalan-test] jkesselm commented on a diff in pull request #1: Move tests/2.7.3 into main test driver framework, document bugzilla test status
Posted by "jkesselm (via GitHub)" <gi...@apache.org>.
jkesselm commented on code in PR #1:
URL: https://github.com/apache/xalan-test/pull/1#discussion_r1189207477
##########
tests/bugzilla/Bugzilla1251.java:
##########
@@ -37,6 +37,19 @@
* Testlet for reproducing Bugzilla reported bugs.
*
* Reported-by: slobo@matavnet.hu
+ *
+ * jkesselm May2023: CONFIRMED OPPOSITE. Despite claim from user, identity transformer reports FAIL (throws exception),
+ * real transformer reports PASS (exception caught and handled as intended).
+ *
+ * In real transformer, ElemCopy catches the SaxException, throws it wrapped in TransformerException;
+ * TransformerImpl catches that, sees that it has an m_serializationHandler. and passes it off to
+ * that. Since that reports checkPass, test is considered good.
+ *
+ * In TransformerIdentityImpl.startElement(), there is no equivalent catch-and-handle for exceptions.
+ *
+ * RECOMMENDATION: SANITY CHECK. User appears to have misstated the problem. If so -- if the description is
Review Comment:
Change pushed.
--
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: dev-unsubscribe@xalan.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xalan.apache.org
For additional commands, e-mail: dev-help@xalan.apache.org