Last Comment Bug 750817 - Some xpcshell tests in netwerk/unit rely on Xpcomlib property and fail on Android
: Some xpcshell tests in netwerk/unit rely on Xpcomlib property and fail on And...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: unspecified
: x86 Android
: -- normal (vote)
: mozilla15
Assigned To: Geoff Brown [:gbrown]
:
:
Mentors:
Depends on:
Blocks: 675039
  Show dependency treegraph
 
Reported: 2012-05-01 11:15 PDT by Geoff Brown [:gbrown]
Modified: 2012-05-04 03:59 PDT (History)
3 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
remove references to Xpcomlib (3.13 KB, patch)
2012-05-01 11:30 PDT, Geoff Brown [:gbrown]
no flags Details | Diff | Splinter Review
remove references to Xpcomlib (3.13 KB, patch)
2012-05-01 12:06 PDT, Geoff Brown [:gbrown]
jduell.mcbugs: review+
Details | Diff | Splinter Review
remove references to Xpcomlib (4.26 KB, patch)
2012-05-01 16:00 PDT, Geoff Brown [:gbrown]
gbrown: review+
Details | Diff | Splinter Review

Description Geoff Brown [:gbrown] 2012-05-01 11:15:10 PDT
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] 2012-05-01 11:30:05 PDT
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.
Comment 2 Jason Duell [:jduell] (needinfo me) 2012-05-01 11:50:49 PDT
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] 2012-05-01 12:06:09 PDT
Created attachment 620012 [details] [diff] [review]
remove references to Xpcomlib

That's a good point...updated to use test_readline6!
Comment 4 Jason Duell [:jduell] (needinfo me) 2012-05-01 13:04:06 PDT
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] 2012-05-01 16:00:15 PDT
Created attachment 620114 [details] [diff] [review]
remove references to Xpcomlib

nits addressed (unused functions removed) r=jduell
Comment 6 Geoff Brown [:gbrown] 2012-05-01 22:02:08 PDT
Passes try: https://tbpl.mozilla.org/?tree=Try&rev=f2cee75d2882
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-05-03 04:00:36 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e56ddb80b935
Comment 8 Ed Morley [:emorley] 2012-05-04 03:59:22 PDT
https://hg.mozilla.org/mozilla-central/rev/e56ddb80b935

Note You need to log in before you can comment on or make changes to this bug.