Create analysis task should use build time as fallback when commit time is not available.
Created attachment 334946 [details] Patch
Comment on attachment 334946 [details] Patch Attachment 334946 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6745646 New failing tests: compositing/debug-borders-dynamic.html
Created attachment 334947 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 334946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334946&action=review > Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 > + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; Can't we just do $result[0]['time'] || $result[0]['build_time'] ?
Comment on attachment 334946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334946&action=review >> Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 >> + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; > > Can't we just do $result[0]['time'] || $result[0]['build_time'] ? No. Logical operator will return true / false in PHP. http://php.net/manual/en/language.operators.logical.php
Comment on attachment 334946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334946&action=review >>> Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 >>> + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; >> >> Can't we just do $result[0]['time'] || $result[0]['build_time'] ? > > No. Logical operator will return true / false in PHP. http://php.net/manual/en/language.operators.logical.php Okay. In that case, do $first_result = array_get($result, 0, array()); return array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time'];
Comment on attachment 334946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334946&action=review >>>> Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 >>>> + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; >>> >>> Can't we just do $result[0]['time'] || $result[0]['build_time'] ? >> >> No. Logical operator will return true / false in PHP. http://php.net/manual/en/language.operators.logical.php > > Okay. In that case, do > $first_result = array_get($result, 0, array()); > return array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']; Sounds a good idea. The 'time' key will always exist as long as there is a matching entry. Changing `array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']` to `$first_result['time'] ? $first_result['time'] : $first_result['build_time']` would make it work.
(In reply to dewei_zhu from comment #7) > Comment on attachment 334946 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334946&action=review > > >>>> Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 > >>>> + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; > >>> > >>> Can't we just do $result[0]['time'] || $result[0]['build_time'] ? > >> > >> No. Logical operator will return true / false in PHP. http://php.net/manual/en/language.operators.logical.php > > > > Okay. In that case, do > > $first_result = array_get($result, 0, array()); > > return array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']; > > Sounds a good idea. The 'time' key will always exist as long as there is a > matching entry. Changing `array_key_exists('time', $first_result) : > $first_result['time'] : $first_result['build_time']` to > `$first_result['time'] ? $first_result['time'] : > $first_result['build_time']` would make it work. Okay. We should be careful for the case when time is 0 and NULL.
Comment on attachment 334946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334946&action=review >>>>>> Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 >>>>>> + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; >>>>> >>>>> Can't we just do $result[0]['time'] || $result[0]['build_time'] ? >>>> >>>> No. Logical operator will return true / false in PHP. http://php.net/manual/en/language.operators.logical.php >>> >>> Okay. In that case, do >>> $first_result = array_get($result, 0, array()); >>> return array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']; >> >> Sounds a good idea. The 'time' key will always exist as long as there is a matching entry. Changing `array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']` to `$first_result['time'] ? $first_result['time'] : $first_result['build_time']` would make it work. > > Okay. We should be careful for the case when time is 0 and NULL. We've already have some code that assumes time is never to be 0. For example: line 29 of create-analysis-task.php `if (!$start_run_time || !$end_run_time || $start_run_time == $end_run_time)`
(In reply to dewei_zhu from comment #9) > Comment on attachment 334946 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334946&action=review > > >>>>>> Websites/perf.webkit.org/public/privileged-api/create-analysis-task.php:97 > >>>>>> + return $result ? ($result[0]['time'] ? $result[0]['time'] : $result[0]['build_time']) : null; > >>>>> > >>>>> Can't we just do $result[0]['time'] || $result[0]['build_time'] ? > >>>> > >>>> No. Logical operator will return true / false in PHP. http://php.net/manual/en/language.operators.logical.php > >>> > >>> Okay. In that case, do > >>> $first_result = array_get($result, 0, array()); > >>> return array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']; > >> > >> Sounds a good idea. The 'time' key will always exist as long as there is a matching entry. Changing `array_key_exists('time', $first_result) : $first_result['time'] : $first_result['build_time']` to `$first_result['time'] ? $first_result['time'] : $first_result['build_time']` would make it work. > > > > Okay. We should be careful for the case when time is 0 and NULL. > > We've already have some code that assumes time is never to be 0. For > example: line 29 of create-analysis-task.php `if (!$start_run_time || > !$end_run_time || $start_run_time == $end_run_time)` I think those are all potential bugs.