WebKit Bugzilla
Attachment 359449 Details for
Bug 193563
: Analyzing a chart that does not exist should not halt whole run-analysis script.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193563-20190117220528.patch (text/plain), 5.07 KB, created by
dewei_zhu
on 2019-01-17 22:05:29 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
dewei_zhu
Created:
2019-01-17 22:05:29 PST
Size:
5.07 KB
patch
obsolete
>Subversion Revision: 240136 >diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog >index 4156c10f0782da67f791486e6846b12d114022bc..908caafa5851a3f806906133bc91b113dd1bc1d5 100644 >--- a/Websites/perf.webkit.org/ChangeLog >+++ b/Websites/perf.webkit.org/ChangeLog >@@ -1,3 +1,21 @@ >+2019-01-17 Dewei Zhu <dewei_zhu@apple.com> >+ >+ Analyzing a chart that does not exist should not halt whole run-analysis script. >+ https://bugs.webkit.org/show_bug.cgi?id=193563 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Halting whole run-analysis script while there is any invalid chart specified in Manifest makes the script fragile. >+ Run-analysis is also responsible for adding retry and sending notification which should not be block by this error. >+ Skipping analyzing the corresponding configuration seems reasonable. >+ >+ * tools/js/measurement-set-analyzer.js: >+ (MeasurementSetAnalyzer.prototype.async._analyzeMeasurementSet): Catch the exception while failing to fetch a measurement set and skip the analysis for this config. >+ * unit-tests/measurement-set-analyzer-tests.js: Added a unit test for this. >+ (mockLogger): >+ (async.set requests): >+ (async): >+ > 2018-12-21 Dewei Zhu <dewei_zhu@apple.com> > > Add UI in analysis task page to show commit testability information. >diff --git a/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js b/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js >index ae3a2c4cbd00f14bd22f2ed3ff71695fd35b235d..b4364ceec51f0b7995038aa72283960b72cf727a 100644 >--- a/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js >+++ b/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js >@@ -49,7 +49,12 @@ class MeasurementSetAnalyzer { > const metric = Metric.findById(measurementSet.metricId()); > const platform = Platform.findById(measurementSet.platformId()); > this._logger.info(`==== "${metric.fullName()}" on "${platform.name()}" ====`); >- await measurementSet.fetchBetween(this._startTime, this._endTime); >+ try { >+ await measurementSet.fetchBetween(this._startTime, this._endTime); >+ } catch (error) { >+ this._logger.warn(`Skipping analysis for "${metric.fullName()}" on "${platform.name()}" as time series does not exit.`); >+ return; >+ } > const currentTimeSeries = measurementSet.fetchedTimeSeries('current', false, false); > const rawValues = currentTimeSeries.values(); > if (!rawValues || rawValues.length < 2) >diff --git a/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js b/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js >index 1f21491c559eea9128694be2134cb816bbba603e..80b7bd39b2a0b377483cda5ffcb43cb27b3e26ae 100644 >--- a/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js >+++ b/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js >@@ -35,10 +35,12 @@ describe('MeasurementSetAnalyzer', () => { > { > const info_logs = []; > const error_logs =[]; >+ const warn_logs =[]; > return { > info: (message) => info_logs.push(message), >+ warn: (message) => warn_logs.push(message), > error: (message) => error_logs.push(message), >- info_logs, error_logs >+ info_logs, warn_logs, error_logs > }; > } > >@@ -89,6 +91,25 @@ describe('MeasurementSetAnalyzer', () => { > assert.deepEqual(logger.error_logs, []); > }); > >+ it('should not analyze if no corresponding time series for a measurement set', async () => { >+ const measurementSet = MeasurementSet.findSet(MockModels.somePlatform.id(), MockModels.someMetric.id(), 5000); >+ const logger = mockLogger(); >+ const measurementSetAnalyzer = new MeasurementSetAnalyzer([measurementSet], 4000, 5000, logger); >+ const analysisPromise = measurementSetAnalyzer.analyzeOnce(measurementSet); >+ assert.equal(requests.length, 1); >+ assert.equal(requests[0].url, `/data/measurement-set-${MockModels.somePlatform.id()}-${MockModels.someMetric.id()}.json`); >+ requests[0].resolve({"platform": MockModels.somePlatform.id(),"metric": MockModels.someMetric.id(),"status":"ConfigurationNotFound"}); >+ >+ try { >+ await analysisPromise; >+ } catch (error) { >+ assert(false, 'Should not throw any exception here'); >+ } >+ assert.deepEqual(logger.info_logs, ['==== "Some test : Some metric" on "Some platform" ====']); >+ assert.deepEqual(logger.warn_logs, [`Skipping analysis for "${MockModels.someMetric.fullName()}" on "${MockModels.somePlatform.name()}" as time series does not exit.`]); >+ assert.deepEqual(logger.error_logs, []); >+ }); >+ > it('should not analyze if there is only one data point in the measurement set', async () => { > const measurementSet = MeasurementSet.findSet(MockModels.somePlatform.id(), MockModels.someMetric.id(), 5000); > const logger = mockLogger();
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193563
:
359449
|
359598