Closed Bug 1361473 Opened 7 years ago Closed 7 years ago

Add filter option for network requests checking for transferred size being larger than specified value

Categories

(DevTools :: Netmonitor, enhancement, P3)

Product:
DevTools
Component:
Netmonitor
Version:
55 Branch
Type:
enhancement

Tracking

(firefox55 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- verified

People

(Reporter: sebo, Assigned: stylizit, Mentored)

References

Details

(Keywords: dev-doc-needed, good-first-bug, Whiteboard: good-first-bug)

Attachments

(1 file, 1 obsolete file)

Bug 1361473 - Add filter option for network requests checking for transferred size being larger than specified value.
59 bytes, text/x-review-board-request
Honza
: review+
Details 
In bug 1041895 the filters 'size' and 'transferred-size' were implemented, which allow to filter the requests by their size order or transferred size order, as well as a 'larger-than' filter allowing to filter them by their size.

In addition to that there should be a 'transferred-larger-than' flag, allowing to filter the requests by their transferred size.

Sebastian
(In reply to Sebastian Zartner [:sebo] from comment #0)
> In bug 1041895 the filters 'size' and 'transferred-size'

Should have been "'size' and 'transferred'".

Sebastian
Keywords: dev-doc-needed
For anyone interested in fixing this bug.

Here is the place where filter logic is implemented:
http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/devtools/client/netmonitor/src/utils/filter-text-utils.js#123

Honza
Mentor: odvarko
Whiteboard: good-first-bug
Hi,
I implemented the new "transferred-larger-than" filter, it seems to pass all netmonitor tests locally.

Should we add a new test case in browser_net_filter-flags.js? It also seems we are missing some tests for other filters such as "transferred" or "size".
Comment on attachment 8865170 [details]
Bug 1361473 - Add filter option for network requests checking for transferred size being larger than specified value.

https://reviewboard.mozilla.org/r/136830/#review139972

Looks good to me.

Thanks for working on this!

Honza
Attachment #8865170 - Flags: review?(odvarko) → review+
(In reply to Matt R from comment #5)
> Hi,
> I implemented the new "transferred-larger-than" filter, it seems to pass all
> netmonitor tests locally.
> 
> Should we add a new test case in browser_net_filter-flags.js? It also seems
> we are missing some tests for other filters such as "transferred" or "size".
Yes please!

Honza
As discussed on IRC, I've updated the patch with the new test case for the new "transferred-larger-than" filter.

And also referred the missing filter flag tests to a separate issue here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1363914
Comment on attachment 8866471 [details]
Bug 1361473 - Added new test case for 'transferred-larger-than' filter;

https://reviewboard.mozilla.org/r/138100/#review141444

Looks & passes just fine for me.

R+

Thanks,
Honza
Attachment #8866471 - Flags: review?(odvarko) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7a8413473cd7
Implemented new 'transferred-larger-than' filter; r=Honza
https://hg.mozilla.org/integration/autoland/rev/7c7707bb58b0
Added new test case for 'transferred-larger-than' filter; r=Honza
Assignee: nobody → matthieu.rigolot
Sorry, this had to be backed out for failing own test:

https://hg.mozilla.org/integration/autoland/rev/b50ea34d8c1a02b4c555c34a561dca49d35a31a0
https://hg.mozilla.org/integration/autoland/rev/badb3704b6e1a80c56ef4e4124dbbbd565e45823

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7c7707bb58b041e528b5f25f6759e792082aaf09&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=98355272&repo=autoland

[task 2017-05-11T11:53:58.627149Z] 11:53:58     INFO - Displayed type: webm
[task 2017-05-11T11:53:58.627935Z] 11:53:58     INFO - Tooltip type: video/webm
[task 2017-05-11T11:53:58.628878Z] 11:53:58     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_filter-flags.js | The displayed type is correct. - 
[task 2017-05-11T11:53:58.630163Z] 11:53:58     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_filter-flags.js | The tooltip type is correct. - 
[task 2017-05-11T11:53:58.630658Z] 11:53:58     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_filter-flags.js | Item shouldn't have 'even' class. - 
[task 2017-05-11T11:53:58.631848Z] 11:53:58     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_filter-flags.js | Item should have 'odd' class. - 
[task 2017-05-11T11:53:58.632732Z] 11:53:58     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_filter-flags.js | The item at index 8 has visibility=false - 
[task 2017-05-11T11:53:58.633661Z] 11:53:58     INFO - Buffered messages finished
[task 2017-05-11T11:53:58.634613Z] 11:53:58     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_filter-flags.js | There should be a specific amount of items in the requests menu. - Got 9, expected 8
[task 2017-05-11T11:53:58.635441Z] 11:53:58     INFO - Stack trace:
[task 2017-05-11T11:53:58.635902Z] 11:53:58     INFO -     chrome://mochikit/content/browser-test.js:test_is:928
[task 2017-05-11T11:53:58.636959Z] 11:53:58     INFO -     chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_filter-flags.js:testContents:246
[task 2017-05-11T11:53:58.637823Z] 11:53:58     INFO -     chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_filter-flags.js:null:229
[task 2017-05-11T11:53:58.638723Z] 11:53:58     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:752:9
[task 2017-05-11T11:53:58.639120Z] 11:53:58     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:672:7
[task 2017-05-11T11:53:58.640076Z] 11:53:58     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
[task 2017-05-11T11:53:58.640577Z] 11:53:58     INFO - Not taking screenshot here: see the one that was previously logged
[task 2017-05-11T11:53:58.641655Z] 11:53:58     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_filter-flags.js | There should be a specific amount of visible items in the requests menu. - Got 9, expected 8
[task 2017-05-11T11:53:58.642585Z] 11:53:58     INFO - Stack trace:
[task 2017-05-11T11:53:58.643383Z] 11:53:58     INFO -     chrome://mochikit/content/browser-test.js:test_is:928
[task 2017-05-11T11:53:58.644264Z] 11:53:58     INFO -     chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_filter-flags.js:testContents:248
[task 2017-05-11T11:53:58.645072Z] 11:53:58     INFO -     chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_filter-flags.js:null:229
[task 2017-05-11T11:53:58.645460Z] 11:53:58     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:752:9
[task 2017-05-11T11:53:58.646339Z] 11:53:58     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:672:7
[task 2017-05-11T11:53:58.647149Z] 11:53:58     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
...and more

Please fix the issues and submit updated patches.
Flags: needinfo?(matthieu.rigolot)
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #12)
> Sorry, this had to be backed out for failing own test:
> Please fix the issues and submit updated patches.

Yes, this is because https://bugzilla.mozilla.org/show_bug.cgi?id=1360473 has been pushed in the meantime. It introduces a new request and updates testContents parameters.

Will rebase and update accordingly.
Flags: needinfo?(matthieu.rigolot)
Bug 1354504 just landed, you might want to rebase on top of that as well.
Attachment #8866471 - Attachment is obsolete: true
(In reply to Tim Nguyen :ntim (not accepting requests until 1st June) from comment #14)
> Bug 1354504 just landed, you might want to rebase on top of that as well.

Thanks for the heads up Tim! I reopened the review request and submitted an updated patch.
Comment on attachment 8865170 [details]
Bug 1361473 - Add filter option for network requests checking for transferred size being larger than specified value.

https://reviewboard.mozilla.org/r/136830/#review142302

::: commit-message-37a5b:1
(Diff revision 2)
> +Bug 1361473 - rebased and updated tests accordingly

Can you change the commit message to:

Bug 1361473 - Add filter option for network requests checking for transferred size being larger than specified value. r=Honza
Comment on attachment 8865170 [details]
Bug 1361473 - Add filter option for network requests checking for transferred size being larger than specified value.

https://reviewboard.mozilla.org/r/136830/#review142302

> Can you change the commit message to:
> 
> Bug 1361473 - Add filter option for network requests checking for transferred size being larger than specified value. r=Honza

Done!
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5cdfc6ce181c
Add filter option for network requests checking for transferred size being larger than specified value. r=Honza
https://hg.mozilla.org/mozilla-central/rev/5cdfc6ce181c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I have reproduced this bug with Nightly 55.0a1 (2017-05-02) on Ubuntu 16.04, 64 bit!

The fix is now verified on Latest Nightly.

Build ID 	20170601100220
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170531]
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: