719846 - dont eval url_mod

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jeff Hammel]

We heavy-handedly eval url_mod:

http://hg.mozilla.org/build/talos/file/79963fc07e5d/bcontroller.py#l79

However, url_mod is always str(int(time.time()*1000)) if it is
anything at all.  We should figure out a way to do this that is more
idiomatic. Note that
http://hg.mozilla.org/build/talos/file/79963fc07e5d/bcontroller.py#l81
won't work at all -- we don't even eval it for the mobile case; if it
is not literally that string (including the complete lack of
whitespace) we completely ignore it for the mobile case.

I would propose

A. we make url_mod be True/False; True -> str(int(time.time()*1000))
B. It'd be nice to rename it to something else
C. Document the damn thing

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

I propose a field called:
timestamp: True/False

In the case we need other url_mod functionality, we can add another field.

Comment 2

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #1)
> I propose a field called:
> timestamp: True/False
> 
> In the case we need other url_mod functionality, we can add another field.

+1, or maybe url_timestamp

Comment 3

[Jeff Hammel]

from bug 704658:

according to the .config files:

#  url_mod     : (OPTIONAL) a bit of code to be evaled and added to the given url during each cycle of the test

ABICT, it is either not set or it is `str(int(time.time()*1000))` for
all cases in the repo.

Comment 5

[Jeff Hammel]

My initial inclination was to have the url parameter be evaluated via string.Template, e.g.

startup_test/startup_test.html?begin=${now}

which bcontroller.py would interpolate to add the current time at invocation.

However, I notice that the whole command is a string in bcontroller.yml:

command: /home/jhammel/firefox/firefox  -profile /tmp/tmprQitL1/profile http://localhost:15707/startup_test/startup_test.html?begin=

We could still use this approach; ABICT it won't break anything.  Or we can use an equally bad hack of using 

url_mod: True # include a timestamp in the url
url_mod: # don't include a timestamp, False or empty value

Any preferences?

Comment 6

[Jeff Hammel]

Attachment: use url_timestamp with True/False values

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 606406 [details] [diff] [review]
use url_timestamp with True/False values

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

nice.  I am glad you went with url_timestamp vs url_mod.

Comment 8

[Mozilla RelEng Bot]

Try run for 81bdc0faa91e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=81bdc0faa91e
Results (out of 72 total builds):
    success: 71
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-81bdc0faa91e

Comment 9

[Jeff Hammel]

try results look good.  So far my android testing has been inconclusive.  tsvg works.  Using ts, which has url_mod: True results in a hang:

(talos)│python run_tests.py ts.yml -n -d
setting debug
DEBUG: running test file ts.yml
reconnecting socket
DEBUG: using testdate: 1331929170
DEBUG: actual date: 1331929170
RETURN:<a href = "http://hg.mozilla.org/mozilla-central/rev/8d1c74566a0b">rev:8d1c74566a0b</a>
qm-pxp01: 
		Started Fri, 16 Mar 2012 13:19:30
Running test ts: 
		Started Fri, 16 Mar 2012 13:19:30
reconnecting socket
DEBUG: operating with platform_type : remote_
pushing directory: /tmp/tmpcHLBOi/profile to /mnt/sdcard/tests/profile
DEBUG: created profile
reconnecting socket
FIRE PROC: 'org.mozilla.fennec  -profile /mnt/sdcard/tests/profile http://10.251.27.101:15707/getInfo.html'
	Screen width/height:1280/800
	colorDepth:16
	Browser inner width/height: 980/800
__metrics__metrics	Screen width/height:1280/800
	colorDepth:16
	Browser inner width/height: 980/696
__metrics__metrics	Screen width/height:1280/800
	colorDepth:16
	Browser inner width/height: 980/800
__metrics__metrics	Screen width/height:1280/800
	colorDepth:16
	Browser inner width/height: 980/800
__metrics__metrics	Screen width/height:1280/800
	colorDepth:16
	Browser inner width/height: 980/800
__metrics__metrics	Screen width/height:1280/800
	colorDepth:16
	Browser inner width/height: 980/696
__metrics__metrics	Screen width/height:0/0
	colorDepth:16
	Browser inner width/height: 0/0

DEBUG: initialized fennec
DEBUG: command line: org.mozilla.fennec  -profile /mnt/sdcard/tests/profile startup_test/startup_test.html?begin=
reconnecting socket
FIRE PROC: 'org.mozilla.fennec  -profile /mnt/sdcard/tests/profile startup_test/startup_test.html?begin=1331929021846'
NOISE: __metrics	Screen width/height:1280/800
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 980/800
NOISE: __metrics__metrics	Screen width/height:1280/800
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 980/696
NOISE: __metrics__metrics	Screen width/height:1280/800
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 980/800
NOISE: __metrics__metrics	Screen width/height:1280/800
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 980/800
NOISE: __metrics__metrics	Screen width/height:1280/800
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 980/800
NOISE: __metrics__metrics	Screen width/height:1280/800
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 980/696
NOISE: __metrics__metrics	Screen width/height:0/0
NOISE: 	colorDepth:16
NOISE: 	Browser inner width/height: 0/0
NOISE: __metrics

I don't know if this is connected to my change or otherwise as my tablet is pretty unpredictable

Comment 10

[Jeff Hammel]

Note that the timestamp is attached correctly: FIRE PROC: 'org.mozilla.fennec  -profile /mnt/sdcard/tests/profile startup_test/startup_test.html?begin=1331929021846'

Comment 11

[Jeff Hammel]

FWIW, I'm often not able to connect to the device at all today.  wireless is being shotty

Comment 12

[Jeff Hammel]

So my patch does do something odd:  here is the URL without my patch applied:

http://10.251.27.101:15707/startup_test/startup_test.html?begin=1331934468148

compare this with comment 10

Comment 13

[Jeff Hammel]

indeed, for some reason with the patch applied I get

tests :
- name: ts
  tpchrome: False
  url : startup_test/startup_test.html?begin=
  url_timestamp : True
  cycles : 10
  timeout: 300
  shutdown : True

Comment 14

[Jeff Hammel]

compare to:

- name: ts
  tpchrome: False
  url : http://10.251.27.101:15707/startup_test/startup_test.html?begin=
  url_mod : str(int(time.time()*1000))
  cycles : 10
  timeout: 300
  shutdown : True

Comment 15

[Jeff Hammel]

ah, whitespace idiocy, wonderful

Comment 16

[Jeff Hammel]

fixing 'url :' -> 'url:' indeed fixes the issue; way to go, PerfConfigurator!

Comment 17

[Jeff Hammel]

pushed with fixed .config files: http://hg.mozilla.org/build/talos/rev/ecef7ac05bfb