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-unsubscr...@xalan.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@xalan.apache.org For additional commands, e-mail: dev-h...@xalan.apache.org