[GitHub] stevedlawrence commented on a change in pull request #170: Fixed CLI integration tests so that they run properly on Windows.
stevedlawrence commented on a change in pull request #170: Fixed CLI integration tests so that they run properly on Windows. URL: https://github.com/apache/incubator-daffodil/pull/170#discussion_r251569006 ## File path: build.sbt ## @@ -139,6 +139,7 @@ lazy val commonSettings = Seq( sourceManaged := baseDirectory.value / "src_managed", resourceManaged := baseDirectory.value / "resource_managed", libraryDependencies ++= Dependencies.common, + parallelExecution in IntegrationTest := false Review comment: Seems reasonble in that case. The change only disables parallel executiong for integration tests (i.e. CLI tests), so it shouldn't affect the time it takes to run all our normal tests. As Josh mentions, we could disable/enable parallel execution based on the operating system, but I think I'd rather evertyhing be as similar as possible on different OS. So if it needs to be disabled on Windows, I'd vote for also disabling on Linux too. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] stevedlawrence commented on a change in pull request #170: Fixed CLI integration tests so that they run properly on Windows.
stevedlawrence commented on a change in pull request #170: Fixed CLI integration tests so that they run properly on Windows. URL: https://github.com/apache/incubator-daffodil/pull/170#discussion_r251411702 ## File path: build.sbt ## @@ -139,6 +139,7 @@ lazy val commonSettings = Seq( sourceManaged := baseDirectory.value / "src_managed", resourceManaged := baseDirectory.value / "resource_managed", libraryDependencies ++= Dependencies.common, + parallelExecution in IntegrationTest := false Review comment: Agreed, I'm not even sure this change is needed at all. I know Mark was running into memory issues when runing on windows and this was one change he made early on. But I'm not sure it really helped. I think the actual fix was changes to DAFFODIL_JAVA_OPTS in TestCLIDebugger. Mark, can you remove this change and see if the memory issues come back? If not, I think we should remove this change completely. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] stevedlawrence commented on a change in pull request #170: Fixed CLI integration tests so that they run properly on Windows.
stevedlawrence commented on a change in pull request #170: Fixed CLI
integration tests so that they run properly on Windows.
URL: https://github.com/apache/incubator-daffodil/pull/170#discussion_r251393696
##
File path:
daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##
@@ -34,13 +34,13 @@ class TestCLITunables2 {
try {
// note: 2>&1 is shell-speak for "connect stderr into stdout"
- val cmd = String.format("""echo "a,b" | %s parse -s %s
-TsuppressSchemaDefinitionWarnings=all 2>&1""", Util.binPath, testSchemaFile)
+ val cmd = String.format("""echo a,b| %s parse -s %s
-TsuppressSchemaDefinitionWarnings=all 2>&1""", Util.binPath, testSchemaFile)
Review comment:
The new ``Util.echoN`` function is actually only needed when the ``-n`` flag
is used. The echo command behaves exactly the same on linux in windows if the
following conditions are met:
1. No flags are provided (i.e. ``-n``)
1. No quotes are needed
1. No spaces exists at the end of the echo string before the pipe
If these are all true, then the only difference between windows and linux
echo is CRLF vs LF, which the schemes in most of these tests handle with
``%NL;``. In this particular test, we needed to remove the quotes and the space
before the pipe to make it work on linux and windows, but the schema is fine
with the newline differences. In cases where the ``-n`` flag was used for
linux, the new ``Util.echoN`` function was created, which creates the right
command to echo without a newline at the end based on the operating system. So
only tests that do not expect a ``%NL;`` need use use the new function.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
