Skip to content

Commit

Permalink
Improve Facebook Click to Load integration tests (#1039)
Browse files Browse the repository at this point in the history 
Improvements to the Facebook Click to Load integration tests,
including:

 1. Reduce flaky failures by waiting until all the required extension
    configuration has been loaded.
 2. Log requests using Puppeteer instead of relying on the test page
    to accurately report the number of allowed/blocked requests.
 3. Test requests to the SDK are redirected properly.
 4. Test that unblocking works correctly and that reloading the page
    restores blocking.

This change also:

 - Renames and expands on the background waiting integration test
   helper module. That way, we can avoid duplicating configuration
   waiting logic.
 - Tweaks the request logging helper module to avoid logging in-flight
   requests that started before a navigation but finished
   afterwards. That race condition was the cause of some test flakiness.
 - Tweaks the YouTube CTL tests to wait for "networkidle0" instead of
   "networkidle2". That means waiting for all requests to complete,
   rather than all but two.
  • Loading branch information
@kzar
kzar committed Feb 9, 2022
1 parent a296f3a commit 1308e91
Show file tree
Hide file tree
Showing 7 changed files with 185 additions and 116 deletions.
18 changes: 9 additions & 9 deletions integration-test/background/atb.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global dbg:false */
const harness = require('../helpers/harness')
const wait = require('../helpers/wait')
const backgroundWait = require('../helpers/backgroundWait')
const fetch = (...args) => import('node-fetch').then(({ default: fetch }) => fetch(...args))

let browser
Expand Down Expand Up @@ -28,7 +28,7 @@ describe('install workflow', () => {
while (!postInstallOpened) {
const urls = await Promise.all(browser.targets().map(target => target.url()))
postInstallOpened = urls.some(url => url.includes('duckduckgo.com/app?post=1'))
await wait.ms(100)
await bgPage.waitForTimeout(100)
}

expect(postInstallOpened).toBeTruthy()
Expand All @@ -47,7 +47,7 @@ describe('install workflow', () => {
* Instead we load up the extension, wait for the ATB process to finish up,
* then reset everything and simulate a fresh install by calling atb.updateATBValues()
*/
await wait.forSetting(bgPage, 'extiSent')
await backgroundWait.forSetting(bgPage, 'extiSent')

await bgPage.evaluate(() => {
dbg.settings.removeSetting('atb')
Expand All @@ -68,7 +68,7 @@ describe('install workflow', () => {
it('should get its ATB param from atb.js when there\'s no install success page', async () => {
// try get ATB params
await bgPage.evaluate(() => dbg.atb.updateATBValues())
await wait.forSetting(bgPage, 'extiSent')
await backgroundWait.forSetting(bgPage, 'extiSent')

const atb = await bgPage.evaluate(() => dbg.settings.getSetting('atb'))
const setAtb = await bgPage.evaluate(() => dbg.settings.getSetting('set_atb'))
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('install workflow', () => {

// try get ATB params again
await bgPage.evaluate(() => dbg.atb.updateATBValues())
await wait.forSetting(bgPage, 'extiSent')
await backgroundWait.forSetting(bgPage, 'extiSent')

const atb = await bgPage.evaluate(() => dbg.settings.getSetting('atb'))
const setAtb = await bgPage.evaluate(() => dbg.settings.getSetting('set_atb'))
Expand Down Expand Up @@ -141,7 +141,7 @@ describe('search workflow', () => {

// wait until normal exti workflow is done so we don't confuse atb.js requests
// when the actual tests run
await wait.forSetting(bgPage, 'extiSent')
await backgroundWait.forSetting(bgPage, 'extiSent')

// grab current atb data
let data = await fetch('https://duckduckgo.com/atb.js')
Expand Down Expand Up @@ -169,7 +169,7 @@ describe('search workflow', () => {
try {
await searchPage.goto('https://duckduckgo.com/?q=test')
// Extra wait for page load
await wait.ms(1000)
await bgPage.waitForTimeout(1000)
} catch (e) {
// goto may time out, but continue test anyway in case of partial load.
}
Expand All @@ -187,7 +187,7 @@ describe('search workflow', () => {
try {
await searchPage.goto('https://duckduckgo.com/?q=test', { waitUntil: 'networkidle0' })
// Extra wait for page load
await wait.ms(1000)
await bgPage.waitForTimeout(1000)
} catch (e) {
// goto may time out, but continue test anyway in case of partial load.
}
Expand All @@ -207,7 +207,7 @@ describe('search workflow', () => {
try {
await searchPage.goto('https://duckduckgo.com/?q=test', { waitUntil: 'networkidle0' })
// Extra wait for page load
await wait.ms(1000)
await bgPage.waitForTimeout(1000)
} catch (e) {
// goto may time out, but continue test anyway in case of partial load.
}
Expand Down
163 changes: 104 additions & 59 deletions integration-test/background/click-to-load-facebook.js
View file
Original file line number Diff line number Diff line change
@@ -1,23 +1,55 @@
const { getDomain } = require('tldts')
const harness = require('../helpers/harness')
const { logPageRequests } = require('../helpers/requests')
const backgroundWait = require('../helpers/backgroundWait')

const testSite = 'https://privacy-test-pages.glitch.me/privacy-protections/click-to-load/'
const facebookDomains = new Set(['facebook.com', 'facebook.net', 'fbcdn.net'])

let browser
let bgPage
let teardown

function summariseFacebookRequests (requests) {
const facebookSDKRedirect = { checked: false, alwaysRedirected: true }
// Note: Requests to the SDK are not included in counts.
let requestCount = 0
let allowCount = 0
let blockCount = 0

for (const request of requests) {
if (facebookDomains.has(getDomain(request.url.href))) {
if (request.url.pathname === '/en_US/sdk.js') {
facebookSDKRedirect.alwaysRedirected = (
facebookSDKRedirect.alwaysRedirected &&
request.status === 'redirected' &&
request.redirectUrl &&
request.redirectUrl.protocol === 'chrome-extension:' &&
request.redirectUrl.pathname.endsWith('/fb-sdk.js')
)
facebookSDKRedirect.checked = true
continue
}

requestCount += 1

if (request.status === 'blocked') {
blockCount += 1
} else {
// Consider anything not blocked as allowed, so that block tests
// don't miss redirected requests or failed requests.
allowCount += 1
}
}
}

return { facebookSDKRedirect, requestCount, allowCount, blockCount }
}

describe('Test Facebook Click To Load', () => {
beforeAll(async () => {
({ browser, bgPage, teardown } = await harness.setup())

await bgPage.waitForFunction(
() => {
console.log('waiting for tds...')
return window.dbg && window.dbg.tds && window.dbg.tds.ClickToLoadConfig
},
{ polling: 100, timeout: 10000 }
)
// wait a little more for the social config to load
await bgPage.waitForTimeout(3000)
await backgroundWait.forAllConfiguration(bgPage)
})

afterAll(async () => {
Expand All @@ -26,61 +58,74 @@ describe('Test Facebook Click To Load', () => {
} catch (e) {}
})

it('CTL: Should block Facebook requests by default', async () => {
it('CTL: Facebook request blocking/redirecting', async () => {
// Open the test page and start logging network requests.
const page = await browser.newPage()

try {
await page.goto(testSite, { waitUntil: 'networkidle2', timeout: 10000 })
} catch (e) {
// timed out waiting for page to load, let's try running the test anyway
console.log(`Timed out: ${e}`)
const pageRequests = []
const clearRequests = logPageRequests(page, pageRequests)

// Initially there should be a bunch of requests. The SDK should
// be redirected to our surrogate but otherwise Facebook requests should
// be blocked.
await page.goto(testSite, { waitUntil: 'networkidle0' })
await page.waitForNetworkIdle({ idleTime: 1000 })
{
const {
requestCount, blockCount, allowCount, facebookSDKRedirect
} = summariseFacebookRequests(pageRequests)

expect(facebookSDKRedirect.checked).toBeTrue()
expect(facebookSDKRedirect.alwaysRedirected).toBeTrue()
expect(requestCount).toBeGreaterThan(5)
expect(blockCount).toEqual(requestCount)
expect(allowCount).toEqual(0)
}

// give it little time just to be sure (facebook widgets can take time to load)
await page.waitForTimeout(4000)
const fbRequestData = await page.evaluate(() => {
return {
requests: document.getElementById('facebook_call_count').innerHTML,
expectedCalls: document.getElementById('facebook_iFrames').innerHTML
}
// Once the user clicks to load the Facebook content, the SDK should be
// loaded and the content should be unblocked.
clearRequests()
const buttonCount = await page.evaluate(() => {
window.buttons =
Array.from(document.querySelectorAll('body > div'))
.map(div => div.shadowRoot && div.shadowRoot.querySelector('button'))
.filter(button => button)
return window.buttons.length
})

expect(Number(fbRequestData.requests)).toBeLessThanOrEqual(Number(fbRequestData.expectedCalls))

try {
await page.close()
} catch (e) {}
})

xit('CTL: Should load Facebook elements on click', async () => {
const page = await browser.newPage()

try {
await page.goto(testSite, { waitUntil: 'networkidle2', timeout: 10000 })
} catch (e) {
// timed out waiting for page to load, let's try running the test anyway
console.log(`Timed out: ${e}`)
for (let i = 0; i < buttonCount; i++) {
const button = await page.evaluateHandle(i => window.buttons[i], i)
await button.click()
}
await page.waitForNetworkIdle({ idleTime: 1000 })
{
const {
requestCount, blockCount, allowCount, facebookSDKRedirect
} = summariseFacebookRequests(pageRequests)

expect(facebookSDKRedirect.checked).toBeTrue()
expect(facebookSDKRedirect.alwaysRedirected).toBeFalse()
expect(requestCount).toBeGreaterThan(5)
expect(blockCount).toEqual(0)
expect(allowCount).toEqual(requestCount)
}
// give it little time just to be sure (facebook widgets can take time to load)
await page.waitForTimeout(4000)

const fbRequestDataBeforeClick = await page.evaluate(() => {
return {
requests: document.getElementById('facebook_call_count').innerHTML
}
})

// click image element to trigger click to load
page.click('div > div > div > button')

await page.waitForTimeout(6000) // FB elements can take a while to load...

const fbRequestDataAfterClick = await page.evaluate(() => {
return {
requests: document.getElementById('facebook_call_count').innerHTML
}
})
// When the page is reloaded, requests should be blocked again.
clearRequests()
await page.reload({ waitUntil: 'networkidle0' })
await page.waitForNetworkIdle({ idleTime: 1000 })
{
const {
requestCount, blockCount, allowCount, facebookSDKRedirect
} = summariseFacebookRequests(pageRequests)

expect(facebookSDKRedirect.checked).toBeTrue()
// FIXME - It seems that requests to the SDK are not reliably
// redirected after the page is reloaded.
// expect(facebookSDKRedirect.alwaysRedirected).toBeTrue()
expect(requestCount).toBeGreaterThan(5)
expect(blockCount).toEqual(requestCount)
expect(allowCount).toEqual(0)
}

expect(Number(fbRequestDataAfterClick.requests)).toBeGreaterThan(Number(fbRequestDataBeforeClick.requests))
page.close()
})
})
31 changes: 9 additions & 22 deletions integration-test/background/click-to-load-youtube.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const harness = require('../helpers/harness')
const { logPageRequests } = require('../helpers/requests')
const { loadTestConfig, unloadTestConfig } = require('../helpers/testConfig')
const backgroundWait = require('../helpers/backgroundWait')

const testSite = 'https://privacy-test-pages.glitch.me/privacy-protections/youtube-click-to-load/'
const testConfig = {
Expand Down Expand Up @@ -125,21 +126,7 @@ function summariseYouTubeRequests (requests) {
describe('Test YouTube Click To Load', () => {
beforeAll(async () => {
({ browser, bgPage, teardown } = await harness.setup())

// Wait for the extension to load its configuration files.
await bgPage.waitForFunction(
() => (
window.dbg &&
window.dbg.tds &&
!window.dbg.tds.isInstalling &&
window.dbg.tds.ClickToLoadConfig &&
window.dbg.tds.tds.trackers &&
window.dbg.tds.tds &&
window.dbg.tds.tds.domains &&
window.dbg.tds.tds.domains['youtube.com'] === 'Google LLC'
),
{ polling: 10 }
)
await backgroundWait.forAllConfiguration(bgPage)

// Overwrite the parts of the configuration needed for our tests.
await loadTestConfig(bgPage, testConfig)
Expand All @@ -158,12 +145,12 @@ describe('Test YouTube Click To Load', () => {
// Open the test page and start logging network requests.
const page = await browser.newPage()
const pageRequests = []
logPageRequests(page, pageRequests)
const clearRequests = logPageRequests(page, pageRequests)

// Initially there should be a bunch of requests. The iframe_api should
// be redirected to our surrogate but otherwise YouTube requests should
// be blocked.
await page.goto(testSite, { waitUntil: 'networkidle2' })
await page.goto(testSite, { waitUntil: 'networkidle0' })
{
const {
youTubeIframeApi, youTubeStandard, youTubeNocookie
Expand All @@ -180,7 +167,7 @@ describe('Test YouTube Click To Load', () => {

// Once the user clicks to load a video, the iframe_api should be loaded
// and the video should be unblocked.
pageRequests.length = 0
clearRequests()
const button = await page.evaluateHandle(
'document.querySelector("div:nth-child(2) > div")' +
'.shadowRoot.querySelector("button")'
Expand All @@ -200,8 +187,8 @@ describe('Test YouTube Click To Load', () => {
}

// When the page is reloaded, requests should be blocked again.
pageRequests.length = 0
await page.reload({ waitUntil: 'networkidle2' })
clearRequests()
await page.reload({ waitUntil: 'networkidle0' })
{
const {
youTubeIframeApi, youTubeStandard, youTubeNocookie
Expand All @@ -217,7 +204,7 @@ describe('Test YouTube Click To Load', () => {
}

// The header button should also unblock YouTube.
pageRequests.length = 0
clearRequests()
const headerButton = await page.evaluateHandle(
'document.querySelector("#short-container > div")' +
'.shadowRoot.querySelector("#DuckDuckGoPrivacyEssentialsCTLElementTitleTextButton")'
Expand All @@ -241,7 +228,7 @@ describe('Test YouTube Click To Load', () => {

it('CTL: YouTube interacting with iframe API', async () => {
const page = await browser.newPage()
await page.goto(testSite, { waitUntil: 'networkidle2' })
await page.goto(testSite, { waitUntil: 'networkidle0' })

// Test the Iframe API controls and events function correctly, even when
// used with an existing video.
Expand Down
8 changes: 4 additions & 4 deletions integration-test/background/onboarding.js
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
const harness = require('../helpers/harness')
const wait = require('../helpers/wait')
const backgroundWait = require('../helpers/backgroundWait')

let browser, bgPage, teardown

describe('onboarding', () => {
beforeEach(async () => {
({ browser, bgPage, teardown } = await harness.setup())

await wait.forSetting(bgPage, 'showWelcomeBanner')
await wait.forSetting(bgPage, 'showCounterMessaging')
await backgroundWait.forSetting(bgPage, 'showWelcomeBanner')
await backgroundWait.forSetting(bgPage, 'showCounterMessaging')
})

afterEach(async () => {
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('onboarding', () => {
window.postMessage({ type: 'rescheduleCounterMessagingRequest' }, window.location.origin)
})

await wait.forSetting(bgPage, 'rescheduleCounterMessagingOnStart')
await backgroundWait.forSetting(bgPage, 'rescheduleCounterMessagingOnStart')
const rescheduleCounterMessagingOnStart = await bgPage.evaluate(() => {
return window.dbg.settings.getSetting('rescheduleCounterMessagingOnStart')
})
Expand Down

0 comments on commit 1308e91

Please sign in to comment.