Attachment 359449 Details for Bug 193563 – Patch

[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:

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();

Actions: View | Formatted Diff | Diff

Attachments on bug 193563: 359449 | 359598