Bug: RcsLite does not work correctly
Specifically it gets very confused over line endings at the end of file, and fails to revert to previous revisions with consistent line endings.
Demonstrated by the testcases on
DevelopBranch.
perl ../bin/TestRunner.pl RcsTests.pm
exporting TWIKI_ASSERTS=1 for extra checking; disable by exporting TWIKI_ASSERTS=0
Assert checking on
.................................................................................................................F......................................F..........
Time: 31 wallclock secs (21.09 usr 2.18 sys + 0.77 cusr 2.07 csys = 26.11 CPU)
!!!FAILURES!!!
Test Results:
Run: 161, Failures: 2, Errors: 0
There were 2 failures:
1) RcsTests.pm:155 - test_ra8Lite(RcsTests)
expected 'one', got 'one
'
2) RcsTests.pm:155 - test_rt8Lite(RcsTests)
expected 'one', got 'one
'
Test was not successful.
RcsLite will still
appear to work correctly, but in fact has different behaviour to
RcsWrap.
There are two possible solutions:
- Fix RcsLite
- Ignore the problem, since it doesn't seem to have bitten any users yet, and recode the tests accordingly.
Any opinions?
--
CrawfordCurrie - 29 Apr 2005
I definatly
want RcsLite to produce the exact same files as
RcsWrap. In fact, one of the tests I was thinking of coding was to to do the same operations using both, and then doing a file comparison.
I'm finding that
RcsWrap has problems at the moment, with results as above, so i'm not sure whats going on at the moment (see the
StoreTests I just commited)
--
SvenDowideit - 29 Apr 2005
On Windows, at least on my machine,
RcsWrap does not retrieve previous versions correctly; instead, an empty string is returned (see
DakarRcsWrapProblem).
RcsLite does retrieve the topic...
--
ThomasWeigert - 30 Apr 2005
Thomas - thats a totally differnt problem - thats the windows version of rcs giving you trouble
Crawford, I think you should find that both
RcsWrap and
RcsLite are currently adding a return to the text - at least thats what my new tests are telling me..
--
SvenDowideit - 30 Apr 2005
It seems to depend on the platform. I'm starting to think that we need to "flex" the spec a bit and ignore trailing newlines.
--
CrawfordCurrie - 08 May 2005
OK, given lack of feedback on this issue I'm making an executive decision. In practical terms, excess newlines at the end of text are a minor irritation at best. it really isn't something to get worked up about. So I'm going to change the tests to be insensitive to changes in whitespace at the end of topic text.
--
CrawfordCurrie - 05 Jun 2005
but what if this is what's adding extra newlines at the end of topics? that's more than a minor irritation as it prevents splicing topics together in various ways
it
sounds like the tests are correct, and the code is wrong.
--
WillNorris - 05 Jun 2005
This absolutely
is what is adding newlines to ends of topics. Yes, the tests are (were) correct and the code is wrong. But I can't fix it, and unless someone else is able to, it isn't going to get fixed, at least not in
DakarRelease.
--
CrawfordCurrie - 06 Jun 2005
well, bumping it off the
DakarRelease certainly isn't going to encourage anyone to work on it! (especially as it wouldn't be on the
DakarRelease page anymore!
) i think this is a pretty high priority item (for the reasons i stated above); i hope
someone will take the time to look into this.
--
WillNorris - 07 Jun 2005
sigh
--
CrawfordCurrie - 09 Jun 2005
I noticed a reference above to comparing
RcsLite and
RcsWrap in tests. Such tests were part of the TWiki code base when I released
RcsLite.
--
JohnTalintyre - 10 Jun 2005
Yep, and your tests formed the basis of the unit test suite we are still using
As I recall, the tests didn't cover the case of newlines at the end of the topic correctly, though the current tests have been extended to do so.
Oh, and the newlines at the end of the topic are not the only equivalence failure. The diff functions in
RcsWrap and
RcsLite report quite different sets of differences (AFAICT they both report
correctly, it's just they report the differences
differently )
--
CrawfordCurrie - 14 Jun 2005
Gawd, this is a can of worms I wish I'd never opened. OK, after much research and the development of 62 different testcases, I think I have found and fixed all the bugs in
RcsLite and
RcsWrap - yes, there were bugs in both.
- RcsWrap context diffs were broken in two ways; first, there was no default number of context lines, which triggered what appears to be a bug in diff; if the unified context is set to 0, it gets line numbers wrong. Also, the way it was coded, it was always missing the first line of the differences, as well. It would still work - mostly - but failed comparison with RcsLite which was actually getting it right in the end. And the type of the date parameter to rlog was wrong.
- RcsLite had major problems with line terminations, necessitating a rewrite of big chunks (and the code shrinking from 1214 lines to 695 ). There were a range of minor inefficiencies and bad coding practices that I eliminated as well. I also implemented
getRevisionAtDate
, which was missing.
- RcsTests has been completely rewritten. It now tests both RcsWrap and RcsLite. It still needs work to make it test RcsFile, though that should arguably be the job of Sven's store interface tester.
SVN 4428
--
CrawfordCurrie - 25 Jun 2005