1360473 - Add tests for network requests cookie-related filter options

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Vangelis Katsikaros]

After https://bugzilla.mozilla.org/show_bug.cgi?id=1354507 it makes sense to add related tests for the set-cookie-* filter options

Comment 1

[Tim Nguyen :ntim]

Tests for flag filtering are here:
https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_filter-flags.js

Comment 2

[Vangelis Katsikaros]

Attachment: Bug 1360473 - Add tests for network requests cookie-related filter options.

Review commit: https://reviewboard.mozilla.org/r/136864/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136864/

Comment 3

[Vangelis Katsikaros]

Comment on attachment 8865196 [details]
Bug 1360473 - Add tests for network requests cookie-related filter options.

https://reviewboard.mozilla.org/r/136864/#review139858

::: devtools/client/netmonitor/test/browser_net_filter-flags.js:141
(Diff revision 1)
>      store.dispatch(Actions.setRequestFilterText(value));
>    }
>  
>    info("Starting test... ");
>  
> -  let waitNetwork = waitForNetworkEvents(monitor, 8);
> +  let waitNetwork = waitForNetworkEvents(monitor, REQUESTS.length);

I guess this doesn't really need to be hardcoded.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8865196 [details]
Bug 1360473 - Add tests for network requests cookie-related filter options.

https://reviewboard.mozilla.org/r/136864/#review139996

LGTM!


> I guess this doesn't really need to be hardcoded.

I don't understand this comment. The number of requests needs to be passed into `waitForNetworkEvents`
You might want to look at:
http://searchfox.org/mozilla-central/rev/8b70b0a5038ef9472fe7c53e04a70c978bb06aed/devtools/client/netmonitor/test/head.js#280

Thanks for working on this!
Honza

Comment 5

[Tim Nguyen :ntim]

(In reply to Vangelis Katsikaros from comment #3)
> Comment on attachment 8865196 [details]
> Bug 1360473 - Add tests for network requests cookie-related filter options.
> 
> https://reviewboard.mozilla.org/r/136864/#review139858
> 
> ::: devtools/client/netmonitor/test/browser_net_filter-flags.js:141
> (Diff revision 1)
> >      store.dispatch(Actions.setRequestFilterText(value));
> >    }
> >  
> >    info("Starting test... ");
> >  
> > -  let waitNetwork = waitForNetworkEvents(monitor, 8);
> > +  let waitNetwork = waitForNetworkEvents(monitor, REQUESTS.length);
> 
> I guess this doesn't really need to be hardcoded.

Agreed, REQUESTS.length is better than 9. I think it was just a copy-paste leftover when I wrote the test.

Comment 6

[Carsten Book [:Tomcat]]

has one open issue that need to be fixed first before we can land this

Comment 7

[Vangelis Katsikaros]

Comment on attachment 8865196 [details]
Bug 1360473 - Add tests for network requests cookie-related filter options.

https://reviewboard.mozilla.org/r/136864/#review139996

Honza, sorry for not being clearer. I saw that originally the number of requests was hardcoded to 8, but I changed it to REQUESTS.length (which would be 9 after this patch). I was wondering if there is a reason for this to be hardcoded.

Tim replied to that with
> Agreed, REQUESTS.length is better than 9. I think it was just a copy-paste leftover when I wrote the test.

Thanks for the input everyone!

Comment 8

[Vangelis Katsikaros]

Carsten, all should be resolved now.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/868098c2cffa
Add tests for network requests cookie-related filter options. r=Honza

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/868098c2cffa