On Mon, Sep 21, 2009 at 6:43 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Joe Swatosh wrote:
>> On Thu, Sep 10, 2009 at 7:32 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>> Author: cmpilato
>>> Date: Thu Sep 10 07:32:11 2009
>>> New Revision: 39219
>
> [...]
>
>> This commit started a Ruby bindings failure. Here is the relevant
>> scrap of test code
>> (I hope this is clear enough to anyone who is not familiar with Ruby,
>> if not please ask):
>>
>> def test_mime_type_detect_with_type_map
>> type_map = {
>> "html" => "text/html",
>> "htm" => "text/html",
>> "png" => "image/png",
>> }
>>
>> nonexistent_html_file = File.join(@tmp_path, "nonexistent.html")
>> assert_raises(Svn::Error::BadFilename) do
>> Svn::Core::MimeType.detect(nonexistent_html_file)
>> end
>> assert_raises(Svn::Error::BadFilename) do
>> Svn::Core::MimeType.detect(nonexistent_html_file, type_map) ###
>> end
>>
>> The second call to Svn::Core::MimeType.detect is no longer raising (marked
>> ### above). I wonder if changing the placement of the block that checks for
>> existence is the culprit? Or if this was an expected change, I will of course
>> update the test.
>
> Sorry about the build breakage. I'm reasonably confident that relocating
> that existence check is, in fact, the culprit. That said, I'm also
> reasonably confident that it was a fair change to make, offering a small
> performance benefit in the majority of cases where it is used with a MIME
> mapping. I could probably be convinced otherwise, though.
>
No worries about the breakage, this is by far not the biggest issue there.
The Ruby bindings tests assert a lot of behavior of various APIs, not just
the bindings themselves (which is one of the reasons the are so often
reporting failures).
Usually, I read the doc string before sending an email to the list to check
if the asserted behavior is documented, not just a result of the current
implementation (sorry I skipped that step this time). Reading it now:
/** Examine utf8-encoded @a file to determine if it can be described by a
* known (as in, known by this function) Multipurpose Internet Mail
* Extension (MIME) type. If so, set @a *mimetype to a character string
* describing the MIME type, else set it to @c NULL.
*
* If not @c NULL, @a mimetype_map is a hash mapping <tt>const char *</tt>
* filename extensions to <tt>const char *</tt> MIME types, and is the
* first source consulted regarding @a file's MIME type.
*
* Use @a pool for any necessary allocations.
*
* @since New in 1.5.
*/
...it doesn't specifically state that the file must exist (although saying that
the file is utf8-encoded might imply it does?). Ah, but when the mimetype_map
is supplied it only checks the extension. Okay, I think I get it. I'll remove
that assertion.
Thanks,
--
Joe
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2397669
Received on 2009-09-22 08:48:53 CEST