629813 - test-file failure: basename should work on paths with no components ("c:" != "")

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Drew Willcoxon :adw]

Attachment: patch A: special-case Windows top-level paths

The file.basename() test fails on Windows when it calls basename() on the top-level path:

error: fail: basename should work on paths with no components ("c:" != "")
info: Traceback (most recent call last):
  File "resource://testpkgs-api-utils-lib/timer.js", line 64, in notifyOnTimeout
    this._callback.apply(null, this._params);
  File "resource://testpkgs-api-utils-lib/unit-test.js", line 223, in null
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://testpkgs-api-utils-lib/unit-test.js", line 248, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://testpkgs-api-utils-lib/unit-test.js", line 266, in start
    this.test.testFunction(this);
  File "resource://testpkgs-api-utils-lib/unit-test-finder.js", line 57, in runTest
    test(runner);
  File "resource://testpkgs-api-utils-tests/test-file.js", line 48, in null
    "basename should work on paths with no components");
  File "resource://testpkgs-api-utils-lib/unit-test.js", line 195, in assertEqual
    this.fail(message);
  File "resource://testpkgs-api-utils-lib/unit-test.js", line 113, in fail
    console.trace();

Apparently the leafName of "c:" is "c:".  My fault for not checking Windows when I wrote the test.

This patch returns the empty string if the leafName is the path itself.  The test passes with it on my Windows VM.  I'm not sure whether it's always the case that if leafName == path, then path has no basename, but I can't think of any other situations where leafName might == path.  I'll attach an alternate patch.

Comment 1

[Drew Willcoxon :adw]

Attachment: patch B: remove guarantee that basename(topLevelPath) == ""

This alternate patch simply removes the guarantee that calling basename() on a top-level path returns the empty string.  That behavior is now undefined.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 508023 [details] [diff] [review]
patch A: special-case Windows top-level paths

Python's os.path.basename() function <http://docs.python.org/library/os.path.html> returns the empty string for the topmost path on both Unix and Windows.

Perl's File::Basename::basename function <http://perldoc.perl.org/File/Basename.html> does what leafName does, but its docs warn that it emulates the quirks of the Unix basename command <http://en.wikipedia.org/wiki/Basename> and recommend against using it.

I think we should do what Python's API designers did and make "basename" work the way developers would expect instead of the way it was apparently prematurely standardized a couple decades ago, so r+ on this first patch!

Comment 3

[Drew Willcoxon :adw]

I agree.

https://github.com/mozilla/addon-sdk/commit/31bf645b236c11255e892a68d4342594b432590a