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/01/22 15:14:34 UTC
[GitHub] [incubator-daffodil] jw3 opened a new pull request #479: Fix schematron integration test ec parsing
jw3 opened a new pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479
Fixes random failures parsing the error code on windows by using getBefore to get the ec string instead of getInput.
DAFFODIL-2461
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765582172
Not to worry, this isn't the first time we've had intermitting errors cause build failures. Glad this one was at least reproducable. That's often difficult with this CI failures.
Though, looks like something is stuck in either github actions or in the coverage report. I have no idea what the issue could be, but it's almost certainly not a Daffodil issue. Unfortunately, these errors aren't all that uncommon either. I usually just cancel the workflow and re-run.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 edited a comment on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 edited a comment on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765544590
> Probably CRLF vs. LF related. I think of '\n' as being LF, not CRLF. I think modern regex has \R which is \n or \r\n.
Yeah I wonder if it is the filter that is applied to the shell
`.withInputFilters(replaceInString("\r\n", "\n"))`
where the regex `$` would be platform specific `\r\n` and that filter changes the stream before it gets to the match.
Nothing to do here right now. Needs some experimentation, it can be addressed in the CLI integrations ticket. Ill document that thought on the line in question here..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 commented on a change in pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 commented on a change in pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#discussion_r562702400
##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/schematron/package.scala
##########
@@ -94,7 +94,7 @@ package object schematron {
private val mustache2 = """\{\{(.+?)}}""".r.unanchored
private val echoEC = s"echo ${if(Util.isWindows) "%errorlevel%" else "$?"}"
- private val matchEC = regexLine("""\d+""")
+ private val matchEC = regexLine(s"""(?<=\\d+)${if(Util.isWindows) eol else "$"}""")
Review comment:
Look behind to find the ec so it is not consumed and is available in `getBefore`
Had to match the end of line differently, `$eol` (`\n`) doesnt match on linux... not sure why.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765557490
+1 thanks for looking into this so quickly!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 commented on a change in pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 commented on a change in pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#discussion_r562702400
##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/schematron/package.scala
##########
@@ -94,7 +94,7 @@ package object schematron {
private val mustache2 = """\{\{(.+?)}}""".r.unanchored
private val echoEC = s"echo ${if(Util.isWindows) "%errorlevel%" else "$?"}"
- private val matchEC = regexLine("""\d+""")
+ private val matchEC = regexLine(s"""(?<=\\d+)${if(Util.isWindows) eol else "$"}""")
Review comment:
Look behind to find the ec so it is not consumed and is available in `getBefore`
Had to match the end of line differently, not entirely sure why `$eol` (`\n`) doesnt match on linux.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 commented on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 commented on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765474324
It appeared that getInput would allow additional characters to be returned after the match position.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 commented on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 commented on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765544590
> Probably CRLF vs. LF related. I think of '\n' as being LF, not CRLF. I think modern regex has \R which is \n or \r\n.
Yeah I wonder if it is the filter that is applied to the shell
`.withInputFilters(replaceInString("\r\n", "\n"))`
where the regex `$` would be platform specific `\r\n` and that filter changes the stream before it gets to the match.
Nothing to do here right now. It can be addressed in the CLI integrations ticket. Ill document that thought.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 commented on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 commented on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765577031
Sure thing. Hate to break the build, especially with intermittent things, now gun-shy of CI runs :fearful:
Speaking of... looks like we are at 14 minutes of generating a coverage report...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#discussion_r562741428
##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/schematron/package.scala
##########
@@ -94,7 +94,7 @@ package object schematron {
private val mustache2 = """\{\{(.+?)}}""".r.unanchored
private val echoEC = s"echo ${if(Util.isWindows) "%errorlevel%" else "$?"}"
- private val matchEC = regexLine("""\d+""")
+ private val matchEC = regexLine(s"""(?<=\\d+)${if(Util.isWindows) eol else "$"}""")
Review comment:
Put this discussion into a comment in front of the offending line, maybe somebody will figure it out in the future. Probably CRLF vs. LF related. I think of '\n' as being LF, not CRLF. I think modern regex has \R which is \n or \r\n.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 edited a comment on pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 edited a comment on pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#issuecomment-765474324
It appeared that getInput would allow additional characters to be returned after the match position.
I had previously avoided using getBefore because it consumed the ec. Solved that using a look behind in the regex.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 merged pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 merged pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] jw3 commented on a change in pull request #479: Fix schematron integration test ec parsing
Posted by GitBox <gi...@apache.org>.
jw3 commented on a change in pull request #479:
URL: https://github.com/apache/incubator-daffodil/pull/479#discussion_r562702400
##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/schematron/package.scala
##########
@@ -94,7 +94,7 @@ package object schematron {
private val mustache2 = """\{\{(.+?)}}""".r.unanchored
private val echoEC = s"echo ${if(Util.isWindows) "%errorlevel%" else "$?"}"
- private val matchEC = regexLine("""\d+""")
+ private val matchEC = regexLine(s"""(?<=\\d+)${if(Util.isWindows) eol else "$"}""")
Review comment:
Look behind to find the so it is not consumed and is available in `getBefore`
Had to match the end of line differently, not entirely sure why `$eol` (`\n`) doesnt match on linux.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org