Re: [O] [Bug] commit 39070b7fc7 breaks babel test
Eric Schulte writes: This test (test-ob/catches-all-references) is from commit c21692506d8, which doesn't have anything to do with newlines (judging from the commit message). To me the more natural behavior is to include the newline in the expansion. Maybe we have discussed this before on list, and decided stripping the newline was preferable, but I don't recall that discussion. I seem to have misremembered. Anyway, when you committed that test you apparently thought it was more natural to leave the trailing newline out. I'm not using such constructs myself, but changing this behaviour more than two years later carries a risk that people have documents that rely on it, for good or bad reasons. Just because behavior ends up being encoded in a test doesn't necessarily mean the behavior is correct. I think as test suites attempt to protect the desired behavior they often end up also protecting incidental behavior of the implementation at the time the test was written. Well, a test test should proof the specification is implemented correctly. This is exactly why test methodology has evolved to unit tests, which mostly avoids testing for incident behaviour or emergent phenomena. Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ Wavetables for the Terratec KOMPLEXER: http://Synth.Stromeko.net/Downloads.html#KomplexerWaves
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
Skip Collins skip.coll...@gmail.com writes: It's been a week and this test still fails. Fixed. Would it make sense to automatically enforce passing all tests before git accepts a change? I for one would strongly oppose this change. This would only make it take longer and thus make it less likely that new code is committed. This is the master branch where development should be fast and experimentation should take place, not the maintenance branch. Best, On Wed, Nov 27, 2013 at 2:58 AM, Achim Gratz strom...@nexgo.de wrote: Hi Eric, this change seems to introduce additional line breaks in the following test: --8---cut here---start-8--- Test test-ob/catches-all-references condition: (ert-test-failed ((should (string= (org-babel-execute-src-block) A literal example\non two lines for me.)) :form (string= A literal example\non two lines\n for me. A literal example\non two lines for me.) :value nil)) FAILED 1/1 test-ob/catches-all-references --8---cut here---end---8--- This seems to happen because the final \n from the babel result is not stripped anymore, pointing to the change in ob-core. IIRC we flip-flopped a few times already with including or not including this final newline, so I don't know whether the code or the test should change. -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
Eric Schulte writes: Fixed. The test was trying to ascertain that an inline babel codeblock didn't force a newline at the end. You've just made it test that there is a trailing newline introduced by the inline babel call, which is clearly against its intent. You could have marked it as a known fail if you were planning to have this as a transitory failure only. I can't find the discussion that led to the former implementation at the moment, but I'm fairly certain that there was a bug report against trailing newlines from inline babel calls. So what is the rationale of re-introducing the trailing newline this time around? Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ Samples for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
On Fri, Dec 6, 2013 at 2:05 PM, Eric Schulte schulte.e...@gmail.com wrote: Skip Collins skip.coll...@gmail.com writes: Would it make sense to automatically enforce passing all tests before git accepts a change? I for one would strongly oppose this change. This would only make it take longer and thus make it less likely that new code is committed. This is the master branch where development should be fast and experimentation should take place, not the maintenance branch. Designating something as an expected failure seems to be a good way to track minor issues that need eventually to be resolved. As a user, I frequently update with make up2 just to avoid getting bitten by stupid errors that might sneak into master. Is it really that much extra work for a developer to run the same command before committing and either fix the error or mark it as a known failure?
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
Hi Skip, Skip Collins skip.coll...@gmail.com writes: Designating something as an expected failure seems to be a good way to track minor issues that need eventually to be resolved. As a user, I frequently update with make up2 just to avoid getting bitten by stupid errors that might sneak into master. Is it really that much extra work for a developer to run the same command before committing and either fix the error or mark it as a known failure? I'm for a relaxed policy here. Users living on the master branch know they are testing an unstable version of Org, and such hiccups just happen. -- Bastien
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
Skip Collins skip.coll...@gmail.com writes: On Fri, Dec 6, 2013 at 2:05 PM, Eric Schulte schulte.e...@gmail.com wrote: Skip Collins skip.coll...@gmail.com writes: Would it make sense to automatically enforce passing all tests before git accepts a change? I for one would strongly oppose this change. This would only make it take longer and thus make it less likely that new code is committed. This is the master branch where development should be fast and experimentation should take place, not the maintenance branch. Designating something as an expected failure seems to be a good way to track minor issues that need eventually to be resolved. As a user, I frequently update with make up2 just to avoid getting bitten by stupid errors that might sneak into master. Is it really that much extra work for a developer to run the same command before committing and either fix the error or mark it as a known failure? If it increases the time taken to make a change by say 25%, then it will result in me addressing only 4/5 as many issues. I personally favor 1. a flexible master branch where we can try things out and spur discussion 2. a setup with less hurdles to committing---it's easy to revert a commit, but impossible recover a commit which is never made Best, -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
Achim Gratz strom...@nexgo.de writes: Eric Schulte writes: Fixed. The test was trying to ascertain that an inline babel codeblock didn't force a newline at the end. What makes this code block inline? You've just made it test that there is a trailing newline introduced by the inline babel call, which is clearly against its intent. You could have marked it as a known fail if you were planning to have this as a transitory failure only. I can't find the discussion that led to the former implementation at the moment, but I'm fairly certain that there was a bug report against trailing newlines from inline babel calls. So what is the rationale of re-introducing the trailing newline this time around? This test (test-ob/catches-all-references) is from commit c21692506d8, which doesn't have anything to do with newlines (judging from the commit message). To me the more natural behavior is to include the newline in the expansion. Maybe we have discussed this before on list, and decided stripping the newline was preferable, but I don't recall that discussion. Just because behavior ends up being encoded in a test doesn't necessarily mean the behavior is correct. I think as test suites attempt to protect the desired behavior they often end up also protecting incidental behavior of the implementation at the time the test was written. Best, Regards, Achim. -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
Re: [O] [Bug] commit 39070b7fc7 breaks babel test
It's been a week and this test still fails. Would it make sense to automatically enforce passing all tests before git accepts a change? On Wed, Nov 27, 2013 at 2:58 AM, Achim Gratz strom...@nexgo.de wrote: Hi Eric, this change seems to introduce additional line breaks in the following test: --8---cut here---start-8--- Test test-ob/catches-all-references condition: (ert-test-failed ((should (string= (org-babel-execute-src-block) A literal example\non two lines for me.)) :form (string= A literal example\non two lines\n for me. A literal example\non two lines for me.) :value nil)) FAILED 1/1 test-ob/catches-all-references --8---cut here---end---8--- This seems to happen because the final \n from the babel result is not stripped anymore, pointing to the change in ob-core. IIRC we flip-flopped a few times already with including or not including this final newline, so I don't know whether the code or the test should change.
[O] [Bug] commit 39070b7fc7 breaks babel test
Hi Eric, this change seems to introduce additional line breaks in the following test: --8---cut here---start-8--- Test test-ob/catches-all-references condition: (ert-test-failed ((should (string= (org-babel-execute-src-block) A literal example\non two lines for me.)) :form (string= A literal example\non two lines\n for me. A literal example\non two lines for me.) :value nil)) FAILED 1/1 test-ob/catches-all-references --8---cut here---end---8--- This seems to happen because the final \n from the babel result is not stripped anymore, pointing to the change in ob-core. IIRC we flip-flopped a few times already with including or not including this final newline, so I don't know whether the code or the test should change. Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ Factory and User Sound Singles for Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
