LoFi integration test should use HasChromeProxyLoFi

tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import logging
 import time
 
 from integration_tests import network_metrics
 from telemetry.page import page_test
 from telemetry.value import scalar
@@ skipping 196 matching lines @@
 
     results.AddValue(scalar.ScalarValue(
         results.current_page, 'via', 'count', via_count))
     results.AddValue(scalar.ScalarValue(
         results.current_page, 'bypass', 'count', bypass_count))
 
   def AddResultsForLoFi(self, tab, results):
     lo_fi_count = 0
 
     for resp in self.IterResponses(tab):
-      if resp.HasChromeProxyViaHeader():
+      if resp.HasChromeProxyLoFi():
         lo_fi_count += 1
       else:
         r = resp.response
         raise ChromeProxyMetricException, (
             '%s: LoFi not in request header.' % (r.url))
 
       cl = resp.content_length

[sclittle, 2015/03/24 01:27:38]

Why write the "cl" variable here if it's overwritten in the next for loop?

[megjablon, 2015/03/24 02:05:29]

Done.

       resource = resp.response.url

[sclittle, 2015/03/24 01:27:38]

Why is this variable set here? Is this variable necessary?

[megjablon, 2015/03/24 02:05:29]

Done.

     results.AddValue(scalar.ScalarValue(

[sclittle, 2015/03/24 01:27:38]

While you're at it, could you also make it so that this test fails if
lo_fi_count is 0? (i.e. if zero responses came back)

[megjablon, 2015/03/24 02:05:29]

Done.

         results.current_page, 'lo_fi', 'count', lo_fi_count))
 
     for resp in self.IterResponses(tab):

[sclittle, 2015/03/24 01:27:38]

Could you combine these two for loops?

[megjablon, 2015/03/24 02:05:29]

Done.

       r = resp.response

[sclittle, 2015/03/24 01:27:38]

unused variable

[megjablon, 2015/03/24 02:05:29]

Done.

       cl = resp.content_length
       ocl = resp.original_content_length
       saving = resp.data_saving_rate * 100
       if cl > 100:
         raise ChromeProxyMetricException, (
             'Image %s is %d bytes. Expecting less than 100 bytes.' %
             (resource, cl))
 
     results.AddValue(scalar.ScalarValue(

[sclittle, 2015/03/24 01:27:38]

Why report the last response's cl, ocl, and saving? Would it make more sense to
instead report the overall cl, ocl, and saving?

If you want to report the overall cl, ocl, and savings, you could call
super(ChromeProxyMetric, self).AddResults(tab, results) like
AddResultsForDataSaving does above; see
https://code.google.com/p/chromium/codesearch#chromium/src/tools/chrome_proxy...
for the implementation of that.

[megjablon, 2015/03/24 02:05:29]

Done.

         results.current_page, 'content_length', 'bytes', cl))
     results.AddValue(scalar.ScalarValue(
         results.current_page, 'original_content_length', 'bytes', ocl))
     results.AddValue(scalar.ScalarValue(
         results.current_page, 'data_saving', 'percent', saving))
 
   def AddResultsForBypass(self, tab, results):
     bypass_count = 0
 
     for resp in self.IterResponses(tab):
@@ skipping 242 matching lines @@
         via_count += 1
     if via_count == 0:
       raise ChromeProxyMetricException, (
           'Expected at least one response through the proxy after the bypass '
           'expired, but zero such responses were received.')
 
     results.AddValue(scalar.ScalarValue(
         results.current_page, 'bypass', 'count', bypass_count))
     results.AddValue(scalar.ScalarValue(
         results.current_page, 'via', 'count', via_count))