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 2022/03/10 23:18:20 UTC

[GitHub] [daffodil] mbeckerle opened a new pull request #772: Clear diagnostics (warnings) on saveParser.

mbeckerle opened a new pull request #772:
URL: https://github.com/apache/daffodil/pull/772


   I'd like suggestions on how to test this. 
   
   DAFFODIL-2267


-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826225866



##########
File path: daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
##########
@@ -592,7 +592,9 @@ object Main {
     try {
       val compiler = Compiler()
       val processor = Timer.getResult("reloading", compiler.reload(savedParser))
-      displayDiagnostics(processor)
+      // note that we do not display the diagnostics that were saved as part of the
+      // saved processor. Those are from compile time, are all warnings, and
+      // are just noise in a production system where we're reloading a parser.

Review comment:
       There's one heap of diagnostics held in static state on the SchemaSetRuntimeData object. This is compilation-time warnings only.
   Runtime diagnostics are gathered in the PState/UState. They have to be because of backtracking (well for parsing anyway) we can't be mutating the diagnostics in the SSRD at runtime since it is shared cross threads. 




-- 
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 merged pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle merged pull request #772:
URL: https://github.com/apache/daffodil/pull/772


   


-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r824931651



##########
File path: daffodil-tdml-processor/src/main/scala/org/apache/daffodil/tdml/processor/DaffodilTDMLDFDLProcessor.scala
##########
@@ -128,11 +128,12 @@ final class TDMLDFDLProcessorFactory private (
         if (useSerializedProcessor) {
           val os = new java.io.ByteArrayOutputStream()
           val output = Channels.newChannel(os)
-          p.save(output)
+          p.save(output, clearDiagnostics = false) // TDML runner wants to preserve all diagnostics

Review comment:
       I am going to try the approach suggested by @stevedlawrence of just removing the display of diagnostics from the CLI's reloading a saved procesor. 




-- 
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] stevedlawrence commented on a change in pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826154366



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)
+        shell.sendLine(cmd)
+        //
+        // Saving the processor should compile and issue SDWs which we should see
+        // in the expected output
+        //
+        shell.expect(contains("""Schema Definition Warning"""))
+        Util.expectExitCode(ExitCode.Success, shell)
+        shell.sendLine("exit")
+        shell.expect(eof)
+      } finally {
+        shell.close()
+      }
+    } while (false) // workaround scala local block bug.

Review comment:
       What is the local block bug?

##########
File path: daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
##########
@@ -592,7 +592,9 @@ object Main {
     try {
       val compiler = Compiler()
       val processor = Timer.getResult("reloading", compiler.reload(savedParser))
-      displayDiagnostics(processor)
+      // note that we do not display the diagnostics that were saved as part of the
+      // saved processor. Those are from compile time, are all warnings, and
+      // are just noise in a production system where we're reloading a parser.

Review comment:
       Just to confirm, we do actually keep compile time and runtime diagnostics completely separate and compile time warnings do not bleed into the ParseResult diagnostics? And I guess the TDML runner combines them both when checking for expected warnings when a test finishes? That's not how I thought it worked, but good to know I was wrong because that seems like a logical way for it to work.

##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)

Review comment:
       Expect keeps track of the stderr and stdout separately, so you don't have to do this redirect. The `expect()` function accepts a an index for which stream to expect from. It defaults to 0 (i.e. stdout) if not specified, but you can speicfy 1 for stderr. So if you remove the redirect, you can change the below expect to be:
   
   ```scala
   shell.expect(1, contains("""Schema Definition Warning"""))
   ```

##########
File path: daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
##########
@@ -592,7 +592,9 @@ object Main {
     try {
       val compiler = Compiler()
       val processor = Timer.getResult("reloading", compiler.reload(savedParser))
-      displayDiagnostics(processor)

Review comment:
       I'm was thinking it might be useful output diagnostics if the -v option is added. But on second thought, probably not--you shouldn't need to enable parse/unparse verbosity just to get information about a saved parser. Probably the right thing is something like a "parser-info" command that can print out metadata stored inside a saved parser. That could include things like daffodil version (which we already have), java version, date/time built, diagnostics, schema information. But that feels like very low priority, especially since most of this information isn't even available at the moment.

##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)
+        shell.sendLine(cmd)
+        //
+        // Saving the processor should compile and issue SDWs which we should see
+        // in the expected output
+        //
+        shell.expect(contains("""Schema Definition Warning"""))
+        Util.expectExitCode(ExitCode.Success, shell)
+        shell.sendLine("exit")
+        shell.expect(eof)
+      } finally {
+        shell.close()
+      }
+    } while (false) // workaround scala local block bug.
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""echo a,b| %s parse -P %s 2>&1""", Util.binPath, compiledProcFilePath)
+        shell.sendLine(cmd)
+        shell.sendLine("exit")
+        val output = shell.expect(eof).getBefore

Review comment:
       This is nice! A good technique to know to just get all output to a string. It's kindof the only good way to check that output doesn't exist.




-- 
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 pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on pull request #772:
URL: https://github.com/apache/daffodil/pull/772#issuecomment-1065346236


   How do I do a CLI test that insists that a particular string is NOT present. 
   
   Can the expect utility just grab all output without hanging waiting for anything, and then I can 
   search it?


-- 
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] stevedlawrence commented on a change in pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826234054



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)
+        shell.sendLine(cmd)
+        //
+        // Saving the processor should compile and issue SDWs which we should see
+        // in the expected output
+        //
+        shell.expect(contains("""Schema Definition Warning"""))
+        Util.expectExitCode(ExitCode.Success, shell)
+        shell.sendLine("exit")
+        shell.expect(eof)
+      } finally {
+        shell.close()
+      }
+    } while (false) // workaround scala local block bug.

Review comment:
       Sounds like a pretty serious bug, and this code works as expected:
   ```scala
   object Main {
     def main(args: Array[String]): Unit = {
       {
         val foo = 1
         println(foo)
       }
       {
         val foo = 2
         println(foo)
       }
     }
   }
   ```
   
   Is there a special case where this doesn't 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: 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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r824923577



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/SchemaSetRuntimeData.scala
##########
@@ -23,20 +23,20 @@ import org.apache.daffodil.processors.unparsers.Unparser
 import org.apache.daffodil.processors.parsers.Parser
 import org.apache.daffodil.processors.TypeCalculatorCompiler.TypeCalcMap
 
-final class SchemaSetRuntimeData(
-  val parser: Parser,
-  val unparser: Unparser,
+final case class SchemaSetRuntimeData(
+  parser: Parser,
+  unparser: Unparser,
   /*
    * Memory of the compiler's warnings. If we save a processor, it's useful to be able
    * to have these warnings.
    */
-  val diagnostics: Seq[Diagnostic],
-  val elementRuntimeData: ElementRuntimeData,
+  diagnostics: Seq[Diagnostic],
+  elementRuntimeData: ElementRuntimeData,
   /*
    * The original variables determined by the schema compiler.
    */
-  variables: VariableMap,
-  val typeCalculators: TypeCalcMap)
+  variables_ : VariableMap,

Review comment:
       I made this a case class to get the automatic copy method. But that also makes all the args into public values. So the _ suffix is to say "not really for public use". 
   
   I think this is moot however, because I'm going to try the approach SL suggested. 




-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r824816939



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
##########
@@ -356,7 +356,14 @@ class DataProcessor private (
   override def newContentHandlerInstance(output: DFDL.Output): DFDL.DaffodilUnparseContentHandler =
     new DaffodilUnparseContentHandler(this, output)
 
-  def save(output: DFDL.Output): Unit = {
+  /**
+   * Save the data processor as serialized stream.
+   * @param output
+   * @param clearDiagnostics When true, clears diagnostics (which must
+   *                         be warnings) so that the serialized processor won't reissue
+   *                         them when reloaded and run.

Review comment:
       Constraint (must be warnings) sounds reasonable to enforce, but I don't see any validation of the diagnostics in save's code.

##########
File path: daffodil-tdml-processor/src/main/scala/org/apache/daffodil/tdml/processor/DaffodilTDMLDFDLProcessor.scala
##########
@@ -128,11 +128,12 @@ final class TDMLDFDLProcessorFactory private (
         if (useSerializedProcessor) {
           val os = new java.io.ByteArrayOutputStream()
           val output = Channels.newChannel(os)
-          p.save(output)
+          p.save(output, clearDiagnostics = false) // TDML runner wants to preserve all diagnostics

Review comment:
       TDML Runner won't do anything with compilation time diagnostics in its usual path anyway.  I noticed that behavior when implementing runtime2's support for returning a processor to TDML Runner.
   
   By the way, I remember that Daffodil's [roadmap](https://cwiki.apache.org/confluence/display/DAFFODIL/Roadmap+for+Upcoming+Releases) has a big refactoring change coming up -- flipping the layering structure of Daffodil so that daffodil-core, with the schema compiler, is split out from the API for generating an executable artifact, and making the schema compiler a library called from a higher layer.  Just worth keeping in mind.

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/SchemaSetRuntimeData.scala
##########
@@ -23,20 +23,20 @@ import org.apache.daffodil.processors.unparsers.Unparser
 import org.apache.daffodil.processors.parsers.Parser
 import org.apache.daffodil.processors.TypeCalculatorCompiler.TypeCalcMap
 
-final class SchemaSetRuntimeData(
-  val parser: Parser,
-  val unparser: Unparser,
+final case class SchemaSetRuntimeData(
+  parser: Parser,
+  unparser: Unparser,
   /*
    * Memory of the compiler's warnings. If we save a processor, it's useful to be able
    * to have these warnings.
    */
-  val diagnostics: Seq[Diagnostic],
-  val elementRuntimeData: ElementRuntimeData,
+  diagnostics: Seq[Diagnostic],
+  elementRuntimeData: ElementRuntimeData,
   /*
    * The original variables determined by the schema compiler.
    */
-  variables: VariableMap,
-  val typeCalculators: TypeCalcMap)
+  variables_ : VariableMap,

Review comment:
       What is the reason for the underscore suffix?




-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826292212



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)
+        shell.sendLine(cmd)
+        //
+        // Saving the processor should compile and issue SDWs which we should see
+        // in the expected output
+        //
+        shell.expect(contains("""Schema Definition Warning"""))
+        Util.expectExitCode(ExitCode.Success, shell)
+        shell.sendLine("exit")
+        shell.expect(eof)
+      } finally {
+        shell.close()
+      }
+    } while (false) // workaround scala local block bug.

Review comment:
       Interesting. I have modified this source, and it compiles fine now. May have been an IDE-related issue. 
   I will put up a separate quick PR to remove this noise. 




-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826229029



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)
+        shell.sendLine(cmd)
+        //
+        // Saving the processor should compile and issue SDWs which we should see
+        // in the expected output
+        //
+        shell.expect(contains("""Schema Definition Warning"""))
+        Util.expectExitCode(ExitCode.Success, shell)
+        shell.sendLine("exit")
+        shell.expect(eof)
+      } finally {
+        shell.close()
+      }
+    } while (false) // workaround scala local block bug.

Review comment:
       You are supposed to be able to just do this:
   ```
   {
       val foo = bar
       ...
   }
   ```
   
   To isolate the scope of foo to just the lines to end of block. 
   But the scala compiler won't let you! Hence a `do { val foo = bar ; .... } while(false)` hack achieves the same thing. 




-- 
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] stevedlawrence commented on a change in pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r824807833



##########
File path: daffodil-tdml-processor/src/main/scala/org/apache/daffodil/tdml/processor/DaffodilTDMLDFDLProcessor.scala
##########
@@ -128,11 +128,12 @@ final class TDMLDFDLProcessorFactory private (
         if (useSerializedProcessor) {
           val os = new java.io.ByteArrayOutputStream()
           val output = Channels.newChannel(os)
-          p.save(output)
+          p.save(output, clearDiagnostics = false) // TDML runner wants to preserve all diagnostics

Review comment:
       As an alternate approach, what are your thoughts on making it so we never clear diagnostics from a DataProcessor, whether it be serialized or not. The benefit is that when you reload a DataProcessor, you still have access to those compile time diagnostics if you want them. So if you want to know if there were any warnings when you compiled some old DataProcessor, you can easily figure that out.
   
   And then to avoid extraneous diagnostics during parse/unparse we change it so DataProcessor diagnostics do not carry over to the ParseResult diagnostics. So there are two completely separate buckets of diagnostics--compile time and runtime. If a user needs access to the compile time diagnostics (like the TDML runner) , it needs to pull them off of the DataProcessor.
   
   In fact, maybe we already do this and the compile-time and run-time diagnosics are already separate? Maybe the fix is to just make it so when we `reload()` we don't output diagnostics from the DataProcessor? In other words, we remove this line:
   
   https://github.com/apache/daffodil/blob/main/daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala#L595
   
   That only affects the CLI. I'm not sure if that is where you were seeing too much diagnostic information.




-- 
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] stevedlawrence commented on pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #772:
URL: https://github.com/apache/daffodil/pull/772#issuecomment-1065356416


   The `expect()` function resturns a [Result](http://javadox.com/net.sf.expectit/expectit-core/0.9.0/net/sf/expectit/Result.html), which has a `getBefore` function to to get the data before the match. Though it just says "part of the data". I'm not sure how back far it goes. Maybe it's some small buffer, maybe it's since the previous expect call? Not sure. If you can `expect` a string that is guaranteed to appear after a warning (if a warning were printed), then that /might/ 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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil] stevedlawrence commented on a change in pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r824873347



##########
File path: daffodil-tdml-processor/src/main/scala/org/apache/daffodil/tdml/processor/DaffodilTDMLDFDLProcessor.scala
##########
@@ -128,11 +128,12 @@ final class TDMLDFDLProcessorFactory private (
         if (useSerializedProcessor) {
           val os = new java.io.ByteArrayOutputStream()
           val output = Channels.newChannel(os)
-          p.save(output)
+          p.save(output, clearDiagnostics = false) // TDML runner wants to preserve all diagnostics

Review comment:
       > TDML Runner won't do anything with compilation time diagnostics in its usual path anyway. I noticed that behavior when implementing runtime2's support for returning a processor to TDML Runner.
   
   If at test has <tdml:errors> or <tdml:warnings> then I think TDML runner should look in both compile time and runtime diagnostics, I think...




-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826229029



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)
+        shell.sendLine(cmd)
+        //
+        // Saving the processor should compile and issue SDWs which we should see
+        // in the expected output
+        //
+        shell.expect(contains("""Schema Definition Warning"""))
+        Util.expectExitCode(ExitCode.Success, shell)
+        shell.sendLine("exit")
+        shell.expect(eof)
+      } finally {
+        shell.close()
+      }
+    } while (false) // workaround scala local block bug.

Review comment:
       You are supposed to be able to just do this:
   ```
   {
       val foo = bar
       ...
   }```
   To isolate the scope of foo to just the lines to end of block. 
   But the scala compiler won't let you! Hence a `do { val foo = bar ; .... } while(false)` hack achieves the same thing. 




-- 
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 #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826238041



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)

Review comment:
       I tried this.
   
   `shell.expect(1,....)`
   
   Very problematic. There are two overloads which take an integer. The first is deprecated and takes a long. That one the long is a timeout. The other takes an int. That one the int is an output stream number. 
   
   I even tried
   
   `shell.expect(1.toInt,...)`
   
   but they still indicate I'm calling the deprecated one where the first arg is a timeout. 
   
   Until they actually remove the deprecated one, 
   I think I will stick with 2>&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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil] stevedlawrence commented on a change in pull request #772: Clear diagnostics (warnings) on saveParser.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #772:
URL: https://github.com/apache/daffodil/pull/772#discussion_r826243661



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -80,4 +82,52 @@ class TestCLITunables2 {
       shell.close()
     }
   }
+
+  @Test def test_CLI_Parsing_ReloadingDoesNotRepeatWarnings(): Unit = {
+    val schemaFile = Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/suppressWarnTest.dfdl.xsd")
+    val testSchemaFile = if (Util.isWindows) Util.cmdConvert(schemaFile) else schemaFile
+    val compiledProcFile = Util.newTempFile("savedProc", ".bin")
+    compiledProcFile.deleteOnExit()
+    val compiledProcFilePath = compiledProcFile.getAbsolutePath()
+    //
+    do {
+      val shell = Util.start("")
+      try {
+        // note: 2>&1 is shell-speak for "connect stderr into stdout"
+        val cmd = String.format("""%s save-parser -s %s %s 2>&1""", Util.binPath, testSchemaFile, compiledProcFilePath)

Review comment:
       Ah, I had the wrong function name. It's `expectIn`, we use it in a handful of places so it should 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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org