You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/12/07 16:50:45 UTC
[GitHub] [daffodil] mphyo21 opened a new pull request #698: Updated constructor that only requires cause exception as an argument…
mphyo21 opened a new pull request #698:
URL: https://github.com/apache/daffodil/pull/698
… to DaffodilUnhandledSAXException
DAFFODIL-2433
Add constructor that only requires cause exception as argument to DaffodilUnhandledSAXException
DAFFODIL-2433
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a change in pull request #698: Updated constructor that only requires cause exception as an argument…
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #698:
URL: https://github.com/apache/daffodil/pull/698#discussion_r764290048
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetInputter.scala
##########
@@ -207,7 +207,8 @@ class SAXInfosetInputter(
}
} catch {
case e: Exception => {
- batchedInfosetEvents(currentIndex).causeError = One(new DaffodilUnhandledSAXException(e.getMessage, e))
+ batchedInfosetEvents(currentIndex).causeError = One(new DaffodilUnhandledSAXException(e))
+
Review comment:
Line 210 isn't covered by any test, which causes the new constructor to be uncovered too.
Does anyone have any idea how to write a test that would cause an exception to escape from dp.unparse at line 202 without being wrapped in an unparseResult.isError?
@mphyo21, if you want to practice your IDE navigation skills, please use your IDE's Go To functionality to find out which code implements the dp.unparse call at line 202 and inform us so we can check if there is any way a test could make an exception escape from that code. If the IDE can't tell you which code implements dp.unparse, you can practice your IDE debugging skills instead - set a breakpoint at line 202, run a test from your IDE, step into the dp.unparse call, and report back to us which code implements that call.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a change in pull request #698: Updated constructor that only requires cause exception as an argument…
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #698:
URL: https://github.com/apache/daffodil/pull/698#discussion_r765920812
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetInputter.scala
##########
@@ -207,7 +207,8 @@ class SAXInfosetInputter(
}
} catch {
case e: Exception => {
Review comment:
Catch of Exception requires documentation. As part of figuring out how to get the change line covered by a test, you should add comments to this scala code as to why catching Exception is ok in this case.
Does anyone (@stevedlawrence @olabusayoT ) know why this is ok? Normally catching Exception or Throwable is almost always an error. Please chime in.
If this in fact should not be Exception, but something narrower, then let's fix that.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/api/DFDLParserUnparser.scala
##########
@@ -263,9 +263,11 @@ object DFDL {
/**
* Used in SAXInfosetEvent.causeError to identify when an unexpected error occurs
*/
- class DaffodilUnhandledSAXException(description: String, cause: Exception)
- extends org.xml.sax.SAXException(description, cause)
+ class DaffodilUnhandledSAXException(description: String, cause: Exception)
+ extends org.xml.sax.SAXException(description, cause) {
+ def this(cause: Exception)= this(null, cause)
Review comment:
A unit test (not a TDML test, just regular unit test) should construct one of these objects so this line is covered.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org