Skip to content

Commit

Permalink
Test changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Micajuine Ho committed Apr 3, 2020
1 parent 2108ac7 commit b6ea443
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 95 deletions.
10 changes: 7 additions & 3 deletions extensions/amp-analytics/0.1/analytics-root.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
} from '../../../src/dom';
import {dev, user, userAssert} from '../../../src/log';
import {getMode} from '../../../src/mode';
import {isArray} from '../../../src/types';
import {isExperimentOn} from '../../../src/experiments';
import {layoutRectLtwh} from '../../../src/layout-rect';
import {map} from '../../../src/utils/object';
import {provideVisibilityManager} from './visibility-manager';
Expand Down Expand Up @@ -340,11 +342,13 @@ export class AnalyticsRoot {
* @param {!Array<string>|string} selectors DOM query selector(s).
* @param {?string=} selectionMethod Allowed values are `null`,
* `'closest'` and `'scope'`.
* @param {boolean=} opt_multiSelectorOn multi-selector expriment
* @return {!Promise<!Array<!AmpElement>>} Array of AMP elements corresponding to the selector if found.
*/
getAmpElements(context, selectors, selectionMethod, opt_multiSelectorOn) {
if (opt_multiSelectorOn) {
getAmpElements(context, selectors, selectionMethod) {
if (
isExperimentOn(this.ampdoc.win, 'visibility-trigger-improvements') &&
isArray(selectors)
) {
userAssert(
!selectionMethod,
'Cannot have selectionMethod %s defined with an array selector.',
Expand Down
24 changes: 10 additions & 14 deletions extensions/amp-analytics/0.1/events.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {dict, hasOwn} from '../../../src/utils/object';
import {getData} from '../../../src/event-helper';
import {getDataParamsFromAttributes} from '../../../src/dom';
import {isArray, isEnumValue, isFiniteNumber} from '../../../src/types';
import {isExperimentOn} from '../../../src/experiments';
import {startsWith} from '../../../src/string';

const SCROLL_PRECISION_PERCENT = 5;
Expand Down Expand Up @@ -1433,9 +1432,6 @@ export class VisibilityTracker extends EventTracker {
let reportWhenSpec = visibilitySpec['reportWhen'];
let createReportReadyPromiseFunc = null;
const unlistenPromises = [];
const multiSelectorVisibilityOn =
isExperimentOn(this.root.ampdoc.win, 'visibility-trigger-improvements') &&
isArray(selector);
if (reportWhenSpec) {
userAssert(
!visibilitySpec['repeat'],
Expand Down Expand Up @@ -1514,8 +1510,7 @@ export class VisibilityTracker extends EventTracker {
.getAmpElements(
context.parentElement || context,
selector,
selectionMethod,
multiSelectorVisibilityOn
selectionMethod
)
.then((elements) => {
for (let i = 0; i < elements.length; i++) {
Expand Down Expand Up @@ -1552,14 +1547,15 @@ export class VisibilityTracker extends EventTracker {
*/
assertUniqueSelectors_(selectors) {
if (isArray(selectors)) {
const filtered = selectors.filter(
(val, index) => selectors.indexOf(val) === index
);
userAssert(
selectors.length === filtered.length,
'Cannot have duplicate selectors in selectors list: %s',
selectors
);
const map = {};
for (let i = 0; i < selectors.length; i++) {
userAssert(
!map[selectors[i]],
'Cannot have duplicate selectors in selectors list: %s',
selectors
);
map[selectors[i]] = selectors[i];
}
}
}

Expand Down
56 changes: 33 additions & 23 deletions extensions/amp-analytics/0.1/test/test-analytics-root.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
VisibilityManagerForEmbed,
} from '../visibility-manager';
import {VisibilityManagerForMApp} from '../visibility-manager-for-mapp';
import {toggleExperiment} from '../../../../src/experiments';

describes.realWin('AmpdocAnalyticsRoot', {amp: 1}, (env) => {
let win;
Expand Down Expand Up @@ -364,68 +365,68 @@ describes.realWin('AmpdocAnalyticsRoot', {amp: 1}, (env) => {
child.classList.add('i-amphtml-element');
child2.classList.add('i-amphtml-element');
child3.classList.add('i-amphtml-element');
toggleExperiment(win, 'visibility-trigger-improvements', true);
});

afterEach(() => {
toggleExperiment(win, 'visibility-trigger-improvements', false);
});

it('should find element and elements by selector', async () => {
child.classList.add('myClass');
child2.classList.add('myClass');
child3.classList.add('notMyClass');
expect(
await root.getAmpElements(body, ['.myClass'], null, true)
await root.getAmpElements(body, ['.myClass'], null)
).to.deep.equal([child, child2]);
// Check that non-experiment works
toggleExperiment(win, 'visibility-trigger-improvements', false);
expect(
await root.getAmpElements(body, '.notMyClass', null, false)
await root.getAmpElements(body, '.notMyClass', null)
).to.deep.equal([child3]);
});

it('should remove duplicate elements found', async () => {
child.id = 'myId';
child.classList.add('myClass');
expect(
await root.getAmpElements(body, ['.myClass', '#myId'], null, true)
).to.deep.equal([child]);
});

it('should remove duplicate selectors', async () => {
child.classList.add('myClass');
expect(
await root.getAmpElements(body, ['.myClass', '.myClass'], null, true)
await root.getAmpElements(body, ['.myClass', '#myId'], null)
).to.deep.equal([child]);
});

it('should ignore special selectors', async () => {
child.classList.add('myClass');
expectAsyncConsoleError(/Element ":host" not found/, 1);
await expect(
root.getAmpElements(body, [':host'], null, true)
root.getAmpElements(body, [':host'], null)
).to.be.rejectedWith(/Element ":host" not found​​​/);
});

it('should handle missing selector for AMP search', async () => {
expectAsyncConsoleError(/Element "#unknown" not found/, 1);
await expect(
root.getAmpElements(body, ['#unknown'], null, true)
root.getAmpElements(body, ['#unknown'], null)
).to.be.rejectedWith(/Element "#unknown" not found​​​/);
});

it('should handle invalid selector', async () => {
expectAsyncConsoleError(/Invalid query selector 12345/, 1);
await expect(
root.getAmpElements(body, [12345], null, true)
root.getAmpElements(body, [12345], null)
).to.be.rejectedWith(/Invalid query selector 12345​​​/);
});

it('should fail if the found element is not AMP for AMP search', async () => {
expectAsyncConsoleError(/required to be an AMP element/, 1);
child.classList.remove('i-amphtml-element');
await expect(
root.getAmpElements(body, ['#child'], null, true)
root.getAmpElements(body, ['#child'], null)
).to.be.rejectedWith(/required to be an AMP element/);
});

it('should fail if selection method is found', async () => {
try {
await root.getAmpElements(body, ['#child'], 'scope', true);
await root.getAmpElements(body, ['#child'], 'scope');
} catch (e) {
expect(e).to.match(
/Cannot have selectionMethod scope defined with an array selector/
Expand Down Expand Up @@ -725,38 +726,47 @@ describes.realWin(
describe('get amp elements', () => {
beforeEach(() => {
child.classList.add('i-amphtml-element');
toggleExperiment(win, 'visibility-trigger-improvements', true);
});

afterEach(() => {
child.classList.add('i-amphtml-element');
toggleExperiment(win, 'visibility-trigger-improvements', false);
});

it('should find all elements by selector', async () => {
const child2 = win.document.createElement('child');
const child3 = win.document.createElement('child');
child.id = 'hi';
child2.id = 'bye';
body.appendChild(child2);
body.appendChild(child3);
child.classList.add('myClass');
child2.classList.add('myClass');
child3.classList.add('notMyClass');
child2.classList.add('i-amphtml-element');
child3.classList.add('i-amphtml-element');
expect(
await root.getAmpElements(body, ['.myClass'], null, true)
).to.deep.equals([child, child2]);
expect(
await root.getAmpElements(body, '.notMyClass', null, false)
).to.deep.equals([child3]);
const e = await root.getAmpElements(body, ['.myClass'], null);

expect(e).to.deep.equals([child, child2]);
// Check that non-experiment version works
// toggleExperiment(win, 'visibility-trigger-improvements', false);
// e = await root.getAmpElements(body, '.notMyClass', null);
// expect(e).to.deep.equals([child3]);
});

it('should handle missing selector for AMP search', async () => {
expectAsyncConsoleError(/Element "#unknown" not found/, 1);
await expect(
root.getAmpElements(body, ['#unknown'], null, true)
root.getAmpElements(body, ['#unknown'], null)
).to.be.rejectedWith(/Element "#unknown" not found​​​/);
});

it('should fail if the found element is not AMP for AMP search', async () => {
expectAsyncConsoleError(/required to be an AMP element/, 1);
child.classList.remove('i-amphtml-element');
await expect(
root.getAmpElements(body, ['#child'], null, true)
root.getAmpElements(body, ['#child'], null)
).to.be.rejectedWith(/required to be an AMP element/);
});
});
Expand Down
99 changes: 44 additions & 55 deletions extensions/amp-analytics/0.1/test/test-events.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1950,6 +1950,7 @@ describes.realWin('Events', {amp: 1}, (env) => {
let saveCallback2;
let eventsSpy;
let res;
let error;

beforeEach(() => {
toggleExperiment(win, 'visibility-trigger-improvements', true);
Expand All @@ -1974,24 +1975,28 @@ describes.realWin('Events', {amp: 1}, (env) => {
});

afterEach(async () => {
[unlisten, unlisten2].forEach((value) => {
if (value) {
expect(value).to.not.be.called;
}
});
expect(res).to.be.a('function');
await res();
[unlisten, unlisten2].forEach((value) => {
if (value) {
expect(value).to.be.calledOnce;
}
});
if (!error) {
[unlisten, unlisten2].forEach((value) => {
if (value) {
expect(value).to.not.be.called;
}
});
expect(res).to.be.a('function');
await res();
[unlisten, unlisten2].forEach((value) => {
if (value) {
expect(value).to.be.calledOnce;
}
});
}

toggleExperiment(win, 'visibility-trigger-improvements', false);
});

it('should fire event per selector', async () => {
config['visibilitySpec'] = {selector: ['.target', '.target2']};
config['visibilitySpec'] = {
selector: ['.target', '.target2'],
};
iniLoadTrackerMock.expects('getRootSignal').twice();
iniLoadTrackerMock
.expects('getElementSignal')
Expand Down Expand Up @@ -2052,6 +2057,18 @@ describes.realWin('Events', {amp: 1}, (env) => {
totalVisibleTime: 15,
});
});

it('should error on duplicate selectors', async () => {
error = true;
config['visibilitySpec'] = {
selector: ['.target', '.target'],
};
expect(() => {
tracker.add(analyticsElement, 'visible', config, eventResolver);
}).to.throw(
/Cannot have duplicate selectors in selectors list: .target/
);
});
});
});

Expand Down Expand Up @@ -2126,28 +2143,6 @@ describes.realWin('Events', {amp: 1}, (env) => {
});

describe('should wait on correct readyPromise', () => {
it('with waitFor default value', () => {
// Default case: selector is not specified
expect(tracker.getReadyPromise(undefined)).to.be.null;
// Default case: waitFor is not specified, no AMP element selected
iniLoadTrackerMock
.expects('getRootSignal')
.returns(Promise.resolve())
.once();
const waitForTracker1 = tracker.getReadyPromise('ini-load');
return waitForTracker1.then(() => {
iniLoadTrackerMock
.expects('getElementSignal')
.withExactArgs('ini-load', target)
.returns(Promise.resolve())
.once();
// Default case: waitFor is not specified, AMP element selected
const promise2 = tracker.getReadyPromise('ini-load', target);
target.signals().signal('ini-load');
return promise2;
});
});

it('with waitFor NONE', () => {
expect(tracker.getReadyPromise('none')).to.be.null;
});
Expand All @@ -2159,16 +2154,13 @@ describes.realWin('Events', {amp: 1}, (env) => {
.twice();
const promise = tracker.getReadyPromise('ini-load');
return promise.then(() => {
const promise1 = tracker.getReadyPromise('ini-load');
return promise1.then(() => {
iniLoadTrackerMock
.expects('getElementSignal')
.withExactArgs('ini-load', target)
.returns(Promise.resolve())
.once();
const promise2 = tracker.getReadyPromise('ini-load', target);
return promise2;
});
iniLoadTrackerMock
.expects('getElementSignal')
.withExactArgs('ini-load', target)
.returns(Promise.resolve())
.once();
const promise2 = tracker.getReadyPromise('ini-load', target);
return promise2;
});
});

Expand All @@ -2187,16 +2179,13 @@ describes.realWin('Events', {amp: 1}, (env) => {
.twice();
const promise = tracker.getReadyPromise('render-start');
return promise.then(() => {
const promise1 = tracker.getReadyPromise('render-start');
return promise1.then(() => {
signalTrackerMock
.expects('getElementSignal')
.withExactArgs('render-start', target)
.returns(Promise.resolve())
.once();
const promise2 = tracker.getReadyPromise('render-start', target);
return promise2;
});
signalTrackerMock
.expects('getElementSignal')
.withExactArgs('render-start', target)
.returns(Promise.resolve())
.once();
const promise2 = tracker.getReadyPromise('render-start', target);
return promise2;
});
});
});
Expand Down

0 comments on commit b6ea443

Please sign in to comment.