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/05/05 18:50:06 UTC

[GitHub] [daffodil] tuxji opened a new pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

tuxji opened a new pull request #554:
URL: https://github.com/apache/daffodil/pull/554


   The unparse test was supposed to fail with the error `Unparse Error: Element {}person does not have a value`, but it passes.
   
   Can we get a new test case that proves the bug still exists?
   
   DAFFODIL-2183
   


-- 
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] [daffodil] tuxji commented on pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   May I have another +1 on the new test?


-- 
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] [daffodil] mbeckerle commented on pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   Tests get split like this when there is a bug in just one newline. (This case) or there is a round trip complexity, which can be due to using newlines (this case too). Folding the tests back together by eliminating these issues would def be cleaner. 


-- 
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] [daffodil] tuxji merged pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   


-- 
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] [daffodil] stevedlawrence commented on pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   +1 looks good!


-- 
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] [daffodil] stevedlawrence commented on pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   I think the bug is saying the expected result is a successful unparse, like what you have, but was instead outputting some error.
   
   Seems to be like this issue was fixed and we just didn't realize it? This bug was opened almost 2 years ago, so that wouldn't surprised me. The tests look like they are correct to me, which would confirm this works as expected now.


-- 
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] [daffodil] tuxji commented on pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   I've changed the tests to use pipe instead of newline as separator, but are the tests worth merging?  They don't seem to differ very much from the other complex_nil tests.
   
   If we still add the tests, I also wonder if we really need to add both a parser test case to literal-value-nils.tdml and an unparser test case to literal-value-nils-unparse.tdml.  Round-tripping will test both the parser and the unparser so I believe it's redundant to add both a parse test case and an unparse test case, right?


-- 
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] [daffodil] stevedlawrence commented on pull request #554: WIP: Add tests to reproduce DAFFODIL-2183, but they pass

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


   Sorry, didn't realize I already gave a +1, still need someone else


-- 
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