Some xpcshell tests in netwerk/unit rely on Xpcomlib property and fail on Android

RESOLVED FIXED in mozilla15

Status

()

Core
Networking
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gbrown, Assigned: gbrown)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
x86
Android
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Some xpcshell tests in netwerk/unit reference the 'Xpcomlib' property simply to find a data file that can be used for the test. On Android, libraries are packaged in the APK, so these tests fail when the library file cannot be opened.

Using a known data file, included in the xpcshell test directories, should be equally effective and will work even in environments that don't have 'Xpcomlib'. (netwerk/test/unit/test_readline.js uses this method)

The tests that currently reference 'Xpcomlib' are:

netwerk/test/unit/test_file_protocol.js
netwerk/test/unit/test_localstreams.js
netwerk/test/unit/test_post.js
Created attachment 620002 [details] [diff] [review]
remove references to Xpcomlib

Instead of using 'Xpcomlib' to find a file, use test_readline1.txt (a simple file that is already present and used by test_readline.js). 

Note that "../unit" is required to allow for unit_ipc tests that wrap the unit tests.
Attachment #620002 - Flags: review?(jduell.mcbugs)
Blocks: 675039
Comment on attachment 620002 [details] [diff] [review]
remove references to Xpcomlib

Review of attachment 620002 [details] [diff] [review]:
-----------------------------------------------------------------

test_readline1.txt is an empty file.  I'm guessing we'd get much better test coverage with a non-empty file like test_readline6.txt?

Not that reading an empty file isn't a useful test data point...
Created attachment 620012 [details] [diff] [review]
remove references to Xpcomlib

That's a good point...updated to use test_readline6!
Attachment #620002 - Attachment is obsolete: true
Attachment #620002 - Flags: review?(jduell.mcbugs)
Attachment #620012 - Flags: review?(jduell.mcbugs)
Comment on attachment 620012 [details] [diff] [review]
remove references to Xpcomlib

Review of attachment 620012 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  +r with a couple nits fixed.

::: netwerk/test/unit/test_localstreams.js
@@ -78,5 @@
>  }
>  
>  function run_test() {
>    // Get a file and a directory in order to do some testing
> -  var file = getDir("XpcomLib");

remove getDir definition from this file too.  It's unused after your change.

::: netwerk/test/unit/test_post.js
@@ -6,5 @@
>  
>  var httpserver = new nsHttpServer();
>  var testpath = "/simple";
>  
> -var testfile = getFile("XpcomLib");

remove getFile definition from this file too.
Attachment #620012 - Flags: review?(jduell.mcbugs) → review+
Created attachment 620114 [details] [diff] [review]
remove references to Xpcomlib

nits addressed (unused functions removed) r=jduell
Attachment #620012 - Attachment is obsolete: true
Attachment #620114 - Flags: review+
Passes try: https://tbpl.mozilla.org/?tree=Try&rev=f2cee75d2882
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/e56ddb80b935
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/e56ddb80b935
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.