humphd/webvtt — pull request #112 by KyleBarnhart (2013-01-24 06:34:30+00:00)
Title: Unit Test Reviews

Fixed the cue identifier unit test.

Fixed the cue timing general unit tests.

Replay to Caitlin Potter:
White space around the timing arrow (-->) is not required for the parser. However since the parser is also validation the syntax I've added the appropriate error messages, just remember that the cue will still be processed normally.

If the first of second line of a cue does not contain the timings then the cue is bad and discarded. I've also added the appropriate errors. Note however the parser does not return a failure. In fact this is how notes are handled.

You are correct that the second and first cues are treated the same. I've fixed that. I was thinking of something else.

I did not rename tests, though I did change "Seperate" to "Separate" on account of spelling, but that should have no real impact. However there was a discrepancy between the .vtt file names and what was reference in the code. I changed that so that it would load the correct files.

Caitlin Potter wrote:
> According to the syntax, it is actually not correct to not have whitespace around the
> cuetime separator, hence we report an error. When the spec changes, this behaviour
> can change.
>
> If the separator is not found, we actually consider that line as the cue ID. However,
> if the line following the cue-ID does not contain cuetimes, we consider it an error
> (WEBVTT_INCOMPLETE, iirc, but don't quote me on that)
>
> Currently, if either timestamp are malformed, the cue is skipped. This is because the
> cuetimes cannot be considered an identifier, and do not contain valid values. The
> second cue is not treated differently from the first in this regard.
>
> Renaming tests or test files is not really review. Something better would be
> justifying those decisions and having someone else address those concerns
> if they are valid.