[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r39219 - trunk/subversion/libsvn_subr

From: Joe Swatosh <joe.swatosh_at_gmail.com>
Date: Mon, 21 Sep 2009 23:48:41 -0700

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

This is an archived mail posted to the Subversion Dev mailing list.