stevedlawrence commented on a change in pull request #429:
URL: https://github.com/apache/incubator-daffodil/pull/429#discussion_r500666860



##########
File path: 
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/SequenceCombinator.scala
##########
@@ -98,8 +123,27 @@ class UnorderedSequence(sq: SequenceTermBase, 
sequenceChildrenArg: Seq[SequenceC
 
   import SeparatedSequenceChildBehavior._
 
+  private lazy val sepMtaGram = sq.delimMTA
+  // Note that we actually only every use one of these depending on depending
+  // on various factors. If there is an optional separtor and a suspension is
+  // used to unaprse that separator, then we cannot use the sepMtaUnaprser
+  // because it results in nested suspensions, which isn't allowed. In that
+  // case, the suspension ends up handling both the optional separator and
+  // alignment using sepMtaAlignmentMaybe.
+  private lazy val (sepMtaAlignmentMaybe, sepMtaUnparserMaybe) =
+    if (sepMtaGram.isEmpty) {
+      (MaybeInt.Nope, Maybe.Nope)
+    } else {
+      (MaybeInt(sq.knownEncodingAlignmentInBits), Maybe(sepMtaGram.unparser))

Review comment:
       I think this is a case where code coverage isn't really an accurae 
representation of test coverage. For example, you could have something like
   ```scala
   if (var1) {
     doTask1()
   }
   
   if (var2) {
     doTask2()
   }
   ```
   If you have one test where ``var1 == true`` and another test where ``var2 == 
true``, we'll get 100% line coverage. But we won't get test coverage where both 
``var1 == true`` and ``var2 == true``. I think that's what's going on here.
   
   This is partly why we need to rely so heavily on functional tests to ensure 
correctness--we have so much code that does one individual thing clearly 
correctly (e.g. a single parser that just parses an int or aligns the bit 
position) but when combined can lead to incorrect results.




----------------------------------------------------------------
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:
[email protected]


Reply via email to