Closed Bug 998071 Opened 6 years ago Closed 6 years ago

implement test coverage for Yahoo search plugin

Categories

(Firefox :: Search, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 32

People

(Reporter: mconnor, Assigned: mconnor)

References

Details

Attachments

(1 file)

Bing/Google test template are a good start.
Attached patch yahooTestsSplinter Review
This is basically a clone of the Bing tests, without the extra purpose params.  I modified the Yahoo plugin to properly use Params so it's easier to test.
Attachment #8408646 - Flags: review?(gavin.sharp)
Attachment #8408646 - Flags: review?(gavin.sharp) → review?(adw)
Comment on attachment 8408646 [details] [diff] [review]
yahooTests

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

These tests don't pass because of the fr parameter commented on below.  r+ with that fixed.  It's an important change but it's small and I trust you to make it correctly :-), so I don't think it's worth another review round.

Bug 962490 adds a search field to about:newtab, so it also updates these behavior tests.  So this Yahoo test will need to check about:newtab, too.  I think we should land this bug first just because it's much smaller, assuming it's ready to land soon, and then I'll update the test in bug 962490.

::: browser/components/search/test/browser_yahoo.js
@@ +12,5 @@
> +function test() {
> +  let engine = Services.search.getEngineByName("Yahoo");
> +  ok(engine, "Yahoo");
> +
> +  let base = "https://search.yahoo.com/search?p=foo&ei=UTF-8&fr=moz35";

fr=moz35 is only set for official branding; otherwise it's fr= (empty string).  You can look at what browser_bing.js does here.

@@ +74,5 @@
> +              purpose: undefined,
> +            },
> +            {
> +              name: "fr",
> +              value: "moz35",

Here too.

::: browser/components/search/test/browser_yahoo_behavior.js
@@ +16,5 @@
> +
> +  let previouslySelectedEngine = Services.search.currentEngine;
> +  Services.search.currentEngine = engine;
> +
> +  let base = "https://search.yahoo.com/search?p=foo&ei=UTF-8&fr=moz35";

Here too.
Attachment #8408646 - Flags: review?(adw) → review+
I landed bug 962490 after all, so assuming it sticks, browser_yahoo_behavior.js should copy over the about:newtab part of the Google/Bing behavior tests.
Since I commented on IRC: the patch in bug 995390 made these apply across all channels, so the tests pass.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fdff43f8f8c
https://hg.mozilla.org/mozilla-central/rev/1fdff43f8f8c
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
You need to log in before you can comment on or make changes to this bug.