Add various tests for the library #114
Conversation
|
Meh. The CI fails for the old version because of language features which were unstable back then. As far as I can see, we have the following options:
The first one would make sense, since we or libs we depend on might end up using now-stable features which were unstable before Thoughts, @matthiasbeyer? |
|
Note: we'll also have to make Travis perform tests for |
|
The Amend that. The problem appears to occur in the looping logic inside I apparently iterated over the Option returned by |
|
Note: I'll extract fixed from this PR into a separate one at some point. |
Instead of iterating over all lines, we iterated only over one element when scanning blocks. This lead to the algorithm only see a block of text or trailers as a sequence of one-line blocks. The resulting behaviour was the `Trailers` type not dumping lines which looked like trailers but were embedded in blocks of texts. This commit fixes the issue by properly iterating over all lines (until the next blank one).
|
Rebased due to CI fix. |
lib/src/message/mod.rs
Outdated
| #[should_panic] | ||
| fn malformed_message_format_check() { | ||
| vec!["Foo bar", "Baz"].into_iter().check_message_format().unwrap(); | ||
| } |
There was a problem hiding this comment.
Why are you not using assert!() in the tests? If things do not panic (in the above tests) they might still not return what we would expect. Especially in this test case, if check_message_format() panics, we get a green test for this - which is clearly not valid!
assert!(vec[..].into_iter().check_message_format().is_err())
would be the way to go here.
|
The |
| pub fn repo(&mut self) -> &mut Repository { | ||
| &mut self.repo | ||
| } | ||
| } |
There was a problem hiding this comment.
What about implementing Drop to remove the repository after the test?
You could even make the removal dependent on an environment variable if you want to keep the repository after the test for debugging purposes.
There was a problem hiding this comment.
I originally planned implementing Drop then introducing TestingRepo. I didn't do it because I hoped it would be unnecessary to remove directories.
Well, keeping testing repos around has the advantage that it can be examined after a failure. If we implement the purging functionality in Drop and let users disable it for a single run, we may still have the repositories around in the next run. This would result in some tests failing.
Btw: if we migrate the testing repo to a key-value storage, we should implement Drop so that all objects are dumped to either stdout or some file, so we end up with something disectable after failed tests.
neithernut
changed the title
|
Note: I originally intended extracting fixes resulting from this test-writing campaign. However, only a single bug surfaced and I think I'll keep the fix in this branch. |
|
@matthiasbeyer: could I get an ACK/comments? |
|
Not earlier than after my last exam + after the Party (so on friday, not earlier). |
|
Blocks #120 due to CI stuff. |
For some of the tests, we require a repository on which we can perform actions.
... by explicitly specifying what to do.
Resolves #78.