557269 - Port old xpcshell tests to the new API

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Dave Townsend [:mossop]]

Copy across all the relevant tests to use the new API.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

I'm sure you're aching for another large patch to review Rob. This basically copies the majority of the old tests for the extension manager over to use the new API. Some have been removed as they are no longer relevant. Some are waiting on bug 556842 getting fixed.

I'm hopeful that you'll only need to skim over these since they are basically what was there before, ugly though some of them are because of that.

Comment 2

[Robert Strong (they/them - no direct email)]

I'm diffing them against the old tests. The following tests are removed and it might be a good thing to validate which ones are no longer relevant, etc.
Only in unit/: test_bug257155.js
Only in unit/: test_bug386153.js
Only in unit/: test_bug391899.js
Only in unit/: test_bug392180.js
Only in unit/: test_bug378216.js
Only in unit/: test_bug394717.js
Only in unit/: test_bug396129.js
Only in unit/: test_bug404024.js
Only in unit/: test_bug417606.js
Only in unit/: test_bug421396.js
Only in unit/: test_bug424107.js
Only in unit/: test_bug424262.js
Only in unit/: test_bug428341.js
Only in unit/: test_bug436207.js
Only in unit/: test_bug449027.js
Only in unit/: test_bug455906.js
Only in unit/: test_bug463819.js
Only in unit/: test_bug474988.js
Only in unit/: test_bug486195.js
Only in unit/: test_bug509194.js
Only in unit/: test_bug511091.js
Only in unit/: test_bug514327_3.js
Only in unit/: test_bug542391.js

Comment 3

[Robert Strong (they/them - no direct email)]

Comment on attachment 437449 [details] [diff] [review]
patch rev 1

The changes to the new api look good

Comment 4

[Dave Townsend [:mossop]]

(In reply to comment #2)
> Only in unit/: test_bug257155.js

Got replaced by test_locale.js in xpcshell/

> Only in unit/: test_bug404024.js
> Only in unit/: test_bug417606.js
> Only in unit/: test_bug424262.js

Tests the component that was used for AMO recommended and searches in the old UI. not yet used in the new UI, the component will be updated and these tests ported at that time (bug )

> Only in unit/: test_bug449027.js
> Only in unit/: test_bug455906.js
> Only in unit/: test_bug463819.js
> Only in unit/: test_bug514327_3.js

Still remains in unit. Tests the blocklist service, will be ported as a part of but bug 556842

> Only in unit/: test_bug509194.js
> Only in unit/: test_bug511091.js

Will be ported as a part of bug 552744.

> Only in unit/: test_bug542391.js

Still remains in unit. Tests the post-update mismatch UI which currently doesn't exist and UX haven't decided exactly what form this will take yet.

> Only in unit/: test_bug378216.js

Tests update security checks, replaced by test_update.js

> Only in unit/: test_bug386153.js

Tests that xpi files dropped into the install location are prompted for on startup. We're dropping support for doing that.

> Only in unit/: test_bug391899.js

Tests that getItemForID returns null for a non-existent add-on. getItemForID doesn't exist anymore.

> Only in unit/: test_bug392180.js

Tests that the %UPDATE_TYPE% parameter is properly escaped, replaced by test_update.js

> Only in unit/: test_bug394717.js

Tests that the old notifications for nsIAddonUpdateCheckListeners get called, not really relevant anymore, the new InstallListeners are tested throughout the other tests.

> Only in unit/: test_bug396129.js

Tests some additional events we added to nsIAddonInstallListener which no longer exist (we might re-add similar events in bug 557716).

> Only in unit/: test_bug421396.js

I misunderstood what this test was and I think I need to add it in again in some form. Filed bug 557897

> Only in unit/: test_bug424107.js

Tests a part of the app mismatches window which doesn't exist any more.

> Only in unit/: test_bug428341.js

Tested that appropriate error messages were displayed when installation failed. Will be implementing a replacement as a part of bug 552965

> Only in unit/: test_bug436207.js

Tests that xpinstall gave the appropriate error code for missing install.rdf. This is now covered in the xpinstall browser chrome tests.

> Only in unit/: test_bug474988.js

Tested that adding/removing multiple install listeners works, this is tested in existing xpcshell tests.

> Only in unit/: test_bug486195.js

Tested that WinRegInstallLocation was cloning the nsIFile instances it gave out. There isn't really a direct way to test this any longer as the install locations are no longer accessible through the APIs, but if it didn't we would be seeing failures in test_registry.js

Comment 5

[Dave Townsend [:mossop]]

http://hg.mozilla.org/mozilla-central/rev/41a9898e597a

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed by the check-in and passing automated tests.