Closed
Bug 1009954
Opened 9 years ago
Closed 7 years ago
implement unit tests for WebappManager
Categories
(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: myk, Assigned: myk)
Details
(Whiteboard: [WebRuntime])
Attachments
(1 file)
11.50 KB,
patch
|
mfinkle
:
feedback+
|
Details | Diff | Splinter Review |
We should implement unit tests for WebappManager, so we can ensure it behaves as expected across internal and external changes. Here's a work in progress that implements unit tests for WebappManager's install, installPackage, _installApk, and _downloadApk methods. It seems wise to get some feedback at this stage to make sure these tests observe Fennec testing best practices and make appropriate changes to the Robocop harness. mfinkle: feel free to redirect this feedback request or request additional feedback if there's someone else who should be looking at Robocop changes, of which there are two: 1. The patch adds two functions to Robocop's do_check_* suite of test functions: * do_check_throws(func, result, stack) A common function in xpcshell derivatives; passes if the test function throws the given result, fails if it doesn't throw or the result doesn't match. * do_check_rejects(promise, result, stack) Like do_check_throws, but for promises; passes if the promise is rejected with the given result, fails if it's resolved or the result doesn't match. The former takes a function, like the other implementations in our tree (which vary; some compare exceptions to Components.results properties). The latter takes the promise itself, which should cover all use cases and makes calling it simpler. But this is the first implementation of such a function in our tree (as far as I can tell), and I don't have a good sense of the breadth of potential use cases, so it's possible that it'll need to evolve over time. 2. robocop_head lazily imported Task.jsm, but in my tests that import only takes 1ms, and do_check_rejects needs to use Task at evaluation time (unless I reimplement it using Promise instead), so it doesn't seem worth loading it lazily, and this patch loads it un-lazily (diligently? eagerly?).
Attachment #8422105 -
Flags: feedback?(mark.finkle)
Assignee | ||
Updated•9 years ago
|
Priority: -- → P1
Comment 1•9 years ago
|
||
Comment on attachment 8422105 [details] [diff] [review] work in progress: tests install, installPackage, _installApk, _downloadApk Oops, I thought I r+ this already. Sorry for the delay.
Attachment #8422105 -
Flags: feedback?(mark.finkle) → feedback+
Comment 2•9 years ago
|
||
s/r+/f+
Assignee | ||
Comment 3•7 years ago
|
||
Per bug 1235869, we're going to disable the Android web runtime, so we won't fix this bug in it. (This is part of a bulk resolution of bugs in the Firefox for Android::Web Apps component, from which I attempted to exclude bugs that are not specific to the runtime, but it's possible that I included one accidentally. If so, I'm sorry, and please reopen the bug!)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•