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

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Geoff Brown [:gbrown]]

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

Comment 1

[Geoff Brown [:gbrown]]

Attachment: 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.

Comment 2

[Jason Duell]

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...

Comment 3

[Geoff Brown [:gbrown]]

Attachment: remove references to Xpcomlib

That's a good point...updated to use test_readline6!

Comment 4

[Jason Duell]

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.

Comment 5

[Geoff Brown [:gbrown]]

Attachment: remove references to Xpcomlib

nits addressed (unused functions removed) r=jduell

Comment 6

[Geoff Brown [:gbrown]]

Passes try: https://tbpl.mozilla.org/?tree=Try&rev=f2cee75d2882

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e56ddb80b935

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e56ddb80b935