Skip to content

Commit

Permalink
🚮 Clean up amp-fit-text server-side experiment (#33204)
Browse files Browse the repository at this point in the history
* Revert "🧪 Enable experiment to upgrade amp-fit-text 0.1 to 1.0 (#32526)"

This reverts commit aace6d6.

* Remove from sources

* Unskip extensions-loading test
  • Loading branch information
caroqliu committed Mar 15, 2021
1 parent ca503aa commit cd9658a
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 62 deletions.
3 changes: 0 additions & 3 deletions build-system/compile/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ const CLOSURE_SRC_GLOBS = [
'extensions/amp-bind/**/*.js',
// Needed to access to Variant interface from other extensions
'extensions/amp-experiment/**/*.js',
// TODO(#32523) Remove this when Bento experiment is done.
// Needed to access across versions
'extensions/amp-fit-text/1.0/*.js',
// Needed to access form impl from other extensions
'extensions/amp-form/**/*.js',
// Needed by amp-facebook-* for the loader logo
Expand Down
8 changes: 1 addition & 7 deletions build-system/global-configs/experiments-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,5 @@
"expiration_date_utc": "2021-06-30",
"define_experiment_constant": "INI_LOAD_INOB"
},
"experimentC": {
"name": "Bento performance analysis experiment",
"environment": "AMP",
"issue": "https://github.com/ampproject/amphtml/issues/32523",
"expiration_date_utc": "2021-12-31",
"define_experiment_constant": "BENTO_AUTO_UPGRADE"
}
"experimentC": {}
}
1 change: 0 additions & 1 deletion build-system/global-configs/experiments-const.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"BENTO_AUTO_UPGRADE": false,
"INI_LOAD_INOB": false,
"NO_SIGNING_RTV": true,
"V1_IMG_DEFERRED_BUILD": false,
Expand Down
3 changes: 0 additions & 3 deletions build-system/test-configs/dep-check-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,6 @@ exports.rules = [
'extensions/amp-youtube/1.0/component.js->extensions/amp-video/1.0/video-iframe.js',
'extensions/amp-youtube/1.0/component.js->extensions/amp-video/1.0/video-wrapper.js',

// Bento amp-fit-text
'extensions/amp-fit-text/0.1/amp-fit-text.js->extensions/amp-fit-text/1.0/base-element.js',

// Amp geo in group enum
'extensions/amp-a4a/0.1/amp-a4a.js->extensions/amp-geo/0.1/amp-geo-in-group.js',
'extensions/amp-consent/0.1/consent-config.js->extensions/amp-geo/0.1/amp-geo-in-group.js',
Expand Down
14 changes: 0 additions & 14 deletions extensions/amp-fit-text/0.1/amp-fit-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
* limitations under the License.
*/

import {BaseElement as BentoFitText} from '../../amp-fit-text/1.0/base-element';
import {CSS} from '../../../build/amp-fit-text-0.1.css';
import {getLengthNumeral, isLayoutSizeDefined} from '../../../src/layout';
import {isAmphtml} from '../../../src/format';
import {px, setStyle, setStyles} from '../../../src/style';
import {throttle} from '../../../src/utils/rate-limit';

Expand Down Expand Up @@ -61,18 +59,6 @@ class AmpFitText extends AMP.BaseElement {
this.textContent_ = '';
}

/** @override */
upgradeCallback() {
if (
BENTO_AUTO_UPGRADE &&
typeof Element.prototype.attachShadow == 'function' &&
isAmphtml(this.element.ownerDocument)
) {
return new BentoFitText(this.element);
}
return null;
}

/** @override */
isLayoutSupported(layout) {
return isLayoutSizeDefined(layout);
Expand Down
34 changes: 15 additions & 19 deletions extensions/amp-fit-text/0.1/test-e2e/test-amp-fit-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,24 @@ describes.endtoend(
controller = env.controller;
});

// TODO(#32523) Remove this when Bento experiment is done.
(BENTO_AUTO_UPGRADE ? it.skip : it)(
'should render in correct font-size',
async () => {
await verifyElementStyles(await selectContentDiv('test1'), {
'font-size': '32px',
});
it('should render in correct font-size', async () => {
await verifyElementStyles(await selectContentDiv('test1'), {
'font-size': '32px',
});

await verifyElementStyles(await selectContentDiv('test2'), {
'font-size': '42px',
'overflow': 'hidden',
});
await verifyElementStyles(await selectContentDiv('test2'), {
'font-size': '42px',
'overflow': 'hidden',
});

await verifyElementStyles(await selectContentDiv('test3'), {
'font-size': '16px',
});
await verifyElementStyles(await selectContentDiv('test3'), {
'font-size': '16px',
});

await verifyElementStyles(await selectContentDiv('test4'), {
'font-size': '20px',
});
}
);
await verifyElementStyles(await selectContentDiv('test4'), {
'font-size': '20px',
});
});

async function selectContentDiv(id) {
return await controller.findElement(
Expand Down
18 changes: 3 additions & 15 deletions test/integration/test-extensions-loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,9 @@ function testLoadOrderFixture(fixtureName, testElements) {
const testElement = fixture.doc.querySelectorAll(testElements[i])[0];
checkElementUpgrade(testElement);
if (testElement.tagName == 'AMP-FIT-TEXT') {
// TODO(#32523) Remove this when Bento experiment is done.
if (BENTO_AUTO_UPGRADE) {
expect(testElement.shadowRoot).to.be.defined;
} else {
expect(
fixture.doc.getElementsByClassName('i-amphtml-fit-text-content')
).to.have.length(1);
}
expect(
fixture.doc.getElementsByClassName('i-amphtml-fit-text-content')
).to.have.length(1);
}
}
});
Expand All @@ -64,13 +59,6 @@ function testLoadOrderFixture(fixtureName, testElements) {
describe('test extensions loading in multiple orders', function () {
this.timeout(15000);

before(function () {
// TODO(#32523) Remove this when Bento experiment is done.
if (BENTO_AUTO_UPGRADE) {
this.skipTest();
}
});

it('one extension, extension loads first, all scripts in header', () => {
return testLoadOrderFixture(
'test/fixtures/script-load-extension-head-v0-head.html',
Expand Down

0 comments on commit cd9658a

Please sign in to comment.