From ab3b4172ceb864f827c8bd1c5cd67aa6c6af18aa Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Mon, 29 Aug 2022 19:52:40 -0400 Subject: [PATCH 1/8] Add tests for mirroring in update-bcd script --- unittest/unit/update-bcd.ts | 200 ++++++++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index ea6019218..944ca9864 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -861,5 +861,205 @@ describe('BCD updater', () => { version_added: null }); }); + + describe('mirror', () => { + const browsers: any = { + chrome: {name: 'Chrome', releases: {85: {}, 86: {}}}, + chrome_android: { + name: 'Chrome Android', + upstream: 'chrome', + releases: {86: {}} + } + }; + + const bcdFromSupport = (support) => ({ + api: {FakeInterface: {__compat: {support}}} + }); + + const mirroringCase = ({support, downstreamResult}) => { + const reports: Report[] = [ + { + __version: '0.3.1', + results: { + 'https://mdn-bcd-collector.appspot.com/tests/': [ + { + name: 'api.FakeInterface', + exposure: 'Window', + result: downstreamResult + } + ] + }, + userAgent: + 'Mozilla/5.0 (Linux; Android 10; SM-G960U) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.5112.97 Mobile Safari/537.36' + } + ]; + const supportMatrix = getSupportMatrix(reports, browsers, []); + const bcd = bcdFromSupport(support); + update(bcd, supportMatrix, {}); + return bcd; + }; + + describe('supported upstream (without flags)', () => { + it('supported in downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: '85'}, + chrome_android: 'mirror' + }, + downstreamResult: true + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: '85'}, + chrome_android: 'mirror' + }) + ); + }); + + it('unsupported in downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: '85'}, + chrome_android: 'mirror' + }, + downstreamResult: false + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: '85'}, + chrome_android: 'mirror' + }) + ); + }); + + it('omitted from downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: '85'}, + chrome_android: 'mirror' + }, + downstreamResult: null + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: '85'}, + chrome_android: 'mirror' + }) + ); + }); + }); + + describe('supported upstream (with flags)', () => { + it('supported in downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: '85', flags: [{}]}, + chrome_android: 'mirror' + }, + downstreamResult: true + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: '85', flags: [{}]}, + chrome_android: [ + {version_added: '86'}, + {flags: [{}], version_added: '85'} + ] + }) + ); + }); + + it('unsupported in downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: '85', flags: [{}]}, + chrome_android: 'mirror' + }, + downstreamResult: false + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: '85', flags: [{}]}, + chrome_android: 'mirror' + }) + ); + }); + + it('omitted from downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: '85', flags: [{}]}, + chrome_android: 'mirror' + }, + downstreamResult: null + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: '85', flags: [{}]}, + chrome_android: 'mirror' + }) + ); + }); + }); + + describe('unsupported upstream', () => { + it('supported in downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: false}, + chrome_android: 'mirror' + }, + downstreamResult: true + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: false}, + chrome_android: 'mirror' + }) + ); + }); + + it('unsupported in downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: false}, + chrome_android: 'mirror' + }, + downstreamResult: false + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: false}, + chrome_android: 'mirror' + }) + ); + }); + + it('omitted from downstream test results', () => { + const actual = mirroringCase({ + support: { + chrome: {version_added: false}, + chrome_android: 'mirror' + }, + downstreamResult: null + }); + assert.deepEqual( + actual, + bcdFromSupport({ + chrome: {version_added: false}, + chrome_android: 'mirror' + }) + ); + }); + }); + }); }); }); From 1dfcbe9ce5d674eab20541fbb4115077ca63211f Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Mon, 29 Aug 2022 21:53:31 -0400 Subject: [PATCH 2/8] Change tests according to perceived intent --- unittest/unit/update-bcd.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index 944ca9864..dbebec3f6 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -912,7 +912,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85'}, - chrome_android: 'mirror' + chrome_android: {version_added: '86'} }) ); }); @@ -929,7 +929,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85'}, - chrome_android: 'mirror' + chrome_android: false }) ); }); @@ -985,7 +985,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85', flags: [{}]}, - chrome_android: 'mirror' + chrome_android: false }) ); }); @@ -1021,7 +1021,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: false}, - chrome_android: 'mirror' + chrome_android: {version_added: '86'} }) ); }); From 1eaa0217c9c2ecaf5394503d99066c99520fecd4 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Tue, 30 Aug 2022 14:19:06 -0400 Subject: [PATCH 3/8] Implement review feedback --- unittest/unit/update-bcd.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index a3dba43ae..87831b53d 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -863,6 +863,8 @@ describe('BCD updater', () => { }); describe('mirror', () => { + const chromeAndroid86UaString = + 'Mozilla/5.0 (Linux; Android 10; SM-G960U) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.5112.97 Mobile Safari/537.36'; const browsers: any = { chrome: {name: 'Chrome', releases: {85: {}, 86: {}}}, chrome_android: { @@ -876,6 +878,14 @@ describe('BCD updater', () => { api: {FakeInterface: {__compat: {support}}} }); + /** + * Create a BCD data structure for an arbitrary web platform feature + * based on support data for Chrome and Chrome Android and test result + * data for Chrome Android. This utility invokes the `update` function + * and is designed to observe the behavior of the "mirror" support value. + * + * @return {BCD} + */ const mirroringCase = ({support, downstreamResult}) => { const reports: Report[] = [ { @@ -889,8 +899,7 @@ describe('BCD updater', () => { } ] }, - userAgent: - 'Mozilla/5.0 (Linux; Android 10; SM-G960U) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.5112.97 Mobile Safari/537.36' + userAgent: chromeAndroid86UaString } ]; const supportMatrix = getSupportMatrix(reports, browsers, []); @@ -912,7 +921,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85'}, - chrome_android: {version_added: '86'} + chrome_android: 'mirror' }) ); }); From f330b8a7684140874bcdd5f9c0623064d69a11fb Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Wed, 31 Aug 2022 13:55:31 -0400 Subject: [PATCH 4/8] Incorporate additional review feedback --- unittest/unit/update-bcd.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index 87831b53d..67171df42 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -975,6 +975,8 @@ describe('BCD updater', () => { bcdFromSupport({ chrome: {version_added: '85', flags: [{}]}, chrome_android: [ + // TODO: Remove flag data when adding enabled-by-default data. + // See https://github.com/mdn/browser-compat-data/pull/16637. {version_added: '86'}, {flags: [{}], version_added: '85'} ] @@ -1030,7 +1032,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: false}, - chrome_android: {version_added: '86'} + chrome_android: {version_added: '≤86'} }) ); }); From 5f3e7f3ed6e7f299e18b04f946dd27be07a8bed7 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Thu, 1 Sep 2022 20:12:12 -0400 Subject: [PATCH 5/8] Correct test expectation --- unittest/unit/update-bcd.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index 67171df42..61b9518d0 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -996,7 +996,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85', flags: [{}]}, - chrome_android: false + chrome_android: 'mirror' }) ); }); From 1cbb4ef9d7ee6d388def4b159d81cad739e1d885 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Wed, 14 Sep 2022 13:53:41 -0400 Subject: [PATCH 6/8] Correct expected value --- unittest/unit/update-bcd.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index 61b9518d0..44f550b7e 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -938,7 +938,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85'}, - chrome_android: false + chrome_android: {version_added: false} }) ); }); From f358663ca7e09adad1ab2ab370c7194549e221e5 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Wed, 14 Sep 2022 14:35:21 -0400 Subject: [PATCH 7/8] Correct implementation --- unittest/unit/update-bcd.ts | 13 ++++--------- update-bcd.ts | 35 +++++++++++++++++++++-------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index d2f99a180..4e1ddcef4 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -1002,7 +1002,7 @@ describe('BCD updater', () => { it('supported in downstream test results', () => { const actual = mirroringCase({ support: { - chrome: {version_added: '85'}, + chrome: {version_added: '86'}, chrome_android: 'mirror' }, downstreamResult: true @@ -1010,7 +1010,7 @@ describe('BCD updater', () => { assert.deepEqual( actual, bcdFromSupport({ - chrome: {version_added: '85'}, + chrome: {version_added: '86'}, chrome_android: 'mirror' }) ); @@ -1064,12 +1064,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: '85', flags: [{}]}, - chrome_android: [ - // TODO: Remove flag data when adding enabled-by-default data. - // See https://github.com/mdn/browser-compat-data/pull/16637. - {version_added: '86'}, - {flags: [{}], version_added: '85'} - ] + chrome_android: {version_added: '86'} }) ); }); @@ -1122,7 +1117,7 @@ describe('BCD updater', () => { actual, bcdFromSupport({ chrome: {version_added: false}, - chrome_android: {version_added: '≤86'} + chrome_android: {version_added: '86'} }) ); }); diff --git a/update-bcd.ts b/update-bcd.ts index 4c8b730d5..94bf6c571 100644 --- a/update-bcd.ts +++ b/update-bcd.ts @@ -299,8 +299,9 @@ export const update = ( continue; } + const support = entry.__compat.support; // Stringified then parsed to deep clone the support statements - const originalSupport = JSON.parse(JSON.stringify(entry.__compat.support)); + const originalSupport = JSON.parse(JSON.stringify(support)); for (const [browser, versionMap] of browserMap.entries()) { if ( @@ -326,11 +327,21 @@ export const update = ( continue; } - let allStatements: InternalSupportStatement | undefined = - entry.__compat.support[browser]; - if (allStatements === 'mirror') { - allStatements = mirror(browser, originalSupport); - } + // Update the support data with a new value. + const persist = (statements: SimpleSupportStatement[]) => { + support[browser] = statements.length === 1 ? statements[0] : statements; + modified = true; + }; + + let allStatements = + (support[browser] as InternalSupportStatement) === 'mirror' + ? mirror(browser, originalSupport) + : // Although non-mirrored support data could be modified in-place, + // working with a cloned version forces the subsequent code to + // explicitly assign it back to the originating data structure. + // This reduces the likelihood of inconsistencies in the handling + // of mirrored and non-mirrored support data. + JSON.parse(JSON.stringify(support[browser] || null)); if (!allStatements) { allStatements = []; @@ -369,11 +380,7 @@ export const update = ( const nonFlagStatements = allStatements.filter( (statement) => !('flags' in statement) ); - entry.__compat.support[browser] = - nonFlagStatements.length === 0 - ? inferredStatement - : [inferredStatement].concat(nonFlagStatements); - modified = true; + persist([inferredStatement, ...nonFlagStatements]); continue; } @@ -439,7 +446,7 @@ export const update = ( ) { simpleStatement.version_added = inferredStatement.version_added.replace('0> ', ''); - modified = true; + persist(allStatements); } } else if ( !( @@ -452,12 +459,12 @@ export const update = ( typeof inferredStatement.version_added === 'string' ? inferredStatement.version_added.replace('0> ', '') : inferredStatement.version_added; - modified = true; + persist(allStatements); } if (typeof inferredStatement.version_removed === 'string') { simpleStatement.version_removed = inferredStatement.version_removed; - modified = true; + persist(allStatements); } } } From b3a5b6473e74106db092cc18932ad8b0d4e928d5 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Tue, 20 Sep 2022 12:56:49 -0400 Subject: [PATCH 8/8] Incorporate `clone` utility function --- unittest/unit/update-bcd.ts | 4 ++-- update-bcd.ts | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/unittest/unit/update-bcd.ts b/unittest/unit/update-bcd.ts index 1f0218326..f2e9efea5 100644 --- a/unittest/unit/update-bcd.ts +++ b/unittest/unit/update-bcd.ts @@ -817,7 +817,7 @@ describe('BCD updater', () => { let bcdCopy; beforeEach(() => { - bcdCopy = JSON.parse(JSON.stringify(bcd)); + bcdCopy = clone(bcd); }); it('normal', () => { @@ -1219,7 +1219,7 @@ describe('BCD updater', () => { firefox: {name: 'Firefox', releases: {92: {}}} } as unknown as Browsers }; - const finalBcd = JSON.parse(JSON.stringify(initialBcd)); + const finalBcd = clone(initialBcd); const report: Report = { __version: '0.3.1', results: { diff --git a/update-bcd.ts b/update-bcd.ts index 94bf6c571..8640290ef 100644 --- a/update-bcd.ts +++ b/update-bcd.ts @@ -65,6 +65,8 @@ export const findEntry = ( return entry; }; +const clone = (value) => JSON.parse(JSON.stringify(value)); + const combineResults = (results: TestResultValue[]): TestResultValue => { let supported: TestResultValue = null; for (const result of results) { @@ -301,7 +303,7 @@ export const update = ( const support = entry.__compat.support; // Stringified then parsed to deep clone the support statements - const originalSupport = JSON.parse(JSON.stringify(support)); + const originalSupport = clone(support); for (const [browser, versionMap] of browserMap.entries()) { if ( @@ -341,7 +343,7 @@ export const update = ( // explicitly assign it back to the originating data structure. // This reduces the likelihood of inconsistencies in the handling // of mirrored and non-mirrored support data. - JSON.parse(JSON.stringify(support[browser] || null)); + clone(support[browser] || null); if (!allStatements) { allStatements = [];