Skip to content

Introduce Safari browser testing #8396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed
26 changes: 26 additions & 0 deletions .github/workflows/test-changed-auth.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,29 @@ jobs:
run: xvfb-run yarn test:changed auth
env:
BROWSERS: 'Firefox'

test-safari:
name: Test Auth on Safari If Changed
runs-on: macos-latest

steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
# This makes Actions fetch all Git history so run-changed script can diff properly.
fetch-depth: 0
- name: Set up Node (20)
uses: actions/setup-node@v3
with:
node-version: 20.x
- name: Test setup and yarn install
run: |
cp config/ci.config.json config/project.json
yarn
- name: build
run: yarn build:changed auth
- name: Run tests on auth changed packages
run: yarn test:changed auth
env:
BROWSERS: 'Safari'

54 changes: 54 additions & 0 deletions .github/workflows/test-changed-firestore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,60 @@ jobs:
env:
BROWSERS: 'Firefox'
EXPERIMENTAL_MODE: true

compat-test-safari:
name: Test Firestore Compatible on Firefox
# Whatever version of Firefox comes with 22.04 is causing Firefox
# startup to hang when launched by karma. Need to look further into
# why.
runs-on: macos-latest
needs: build
if: ${{ needs.build.outputs.changed == 'true'}}
steps:
- name: Set up Node (20)
uses: actions/setup-node@v3
with:
node-version: 20.x
- name: Download build archive
uses: actions/download-artifact@v3
with:
name: build.tar.gz
- name: Unzip build artifact
run: tar xf build.tar.gz
- name: Test setup and yarn install
run: cp config/ci.config.json config/project.json
- name: Run compat tests
run: cd packages/firestore-compat && yarn run test:ci
env:
BROWSERS: 'Safari'

test-safari:
name: Test Firestore on Safari
strategy:
matrix:
test-name: ["test:browser", "test:lite:browser", "test:browser:prod:nameddb", "test:lite:browser:nameddb"]
runs-on: macos-latest
needs: build
if: ${{ needs.build.outputs.changed == 'true'}}
steps:
- name: Download build archive
uses: actions/download-artifact@v3
with:
name: build.tar.gz
- name: Unzip build artifact
run: tar xf build.tar.gz
- name: Set up Node (20)
uses: actions/setup-node@v3
with:
node-version: 20.x
- name: Test setup and yarn install
run: cp config/ci.config.json config/project.json
- name: Run tests
run: cd packages/firestore && yarn run ${{ matrix.test-name }}
env:
BROWSERS: 'Safari'
EXPERIMENTAL_MODE: true


# A job that fails if any required job in the test matrix fails,
# to be used as a required check for merging.
Expand Down
25 changes: 25 additions & 0 deletions .github/workflows/test-changed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,28 @@ jobs:
run: xvfb-run yarn test:changed core
env:
BROWSERS: 'Firefox'

test-safari:
name: Test Packages With Changed Files in Safari
runs-on: macos-latest

steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Set up Node (20)
uses: actions/setup-node@v3
with:
node-version: 20.x
- name: Test setup and yarn install
run: |
cp config/ci.config.json config/project.json
yarn
- name: build
run: yarn build
- name: Run tests on changed packages
run: npx lerna run test:browser --ignore @firebase/firestore --ignore @firebase/auth --ignore firebase-messaging-integration-test
env:
BROWSERS: 'Safari'

68 changes: 64 additions & 4 deletions config/karma.base.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,66 @@ const path = require('path');
const webpackTestConfig = require('./webpack.test');
const { argv } = require('yargs');

function determineBrowsers() {
const supportedBrowsers = ['ChromeHeadless', 'Safari', 'Firefox'];

if (process.env.BROWSERS) {
const browsers = process.env.BROWSERS.split(',');

const validBrowsers = browsers.filter(browser =>
supportedBrowsers.includes(browser)
);
if (validBrowsers.length === 0) {
console.error(
`The \'BROWSER\' environment variable was set, but no supported browsers were listed. The supported browsers are ${JSON.stringify(
supportedBrowsers
)}.`
);
return [];
}

if (browsers.includes('Safari') && process.platform !== 'darwin') {
console.error(
"The 'BROWSER' environment variable includes 'Safari', which is not supported on this platform. The only supported platform is darwin."
);
return [];
}

if (browsers.includes('Safari')) {
console.log(
"\x1b[38;2;255;165;0mSafari browser testing has been enabled. This requires Full Disk Access be granted to the executor. To grant Full Disk Access on macOS > 13, visit 'System Settings' > 'Privacy & Security' > 'Full Disk Access'.\x1b[0m"
); // Log in orange
}

return validBrowsers;
} else {
/**
* By default we only run the Chrome tests locally since the Safari launcher has some quirks
* that make local testing annoying:
* - Tabs from previous test runs are restored, causing the tests to be ran once on each tab.
* To prevent this, Safari has to be manually re-opened and then quit after every test run.
* - Full Disk Access has to be manually enabled
*
* Running the browser tests in Chrome should catch most browser bugs. If that's not the case,
* the bugs will be caught when the browser tests are ran on Safari in CI.
*/
console.log(
"The 'BROWSER' environment variable is undefined. Defaulting to 'ChromeHeadless'."
);
return ['ChromeHeadless'];
}
}

const config = {
// See: https://karma-runner.github.io/6.4/config/plugins.html#loading-plugins
plugins: [
// We use our own custom Safari launcher plugin since https://github.com/karma-runner/karma-safari-launcher
// does not work and is not maintained.
require('../scripts/ci-test/karmaSafariLauncher.js'),
// Include all other plugins from our npm modules
'karma-*'
],

// disable watcher
autoWatch: false,

Expand Down Expand Up @@ -57,10 +116,11 @@ const config = {
// changes
autoWatch: false,

// start these browsers
// available browser launchers:
// https://npmjs.org/browse/keyword/karma-launcher
browsers: process.env?.BROWSERS?.split(',') ?? ['ChromeHeadless'],
// Browsers to launch for testing
// To use a custom set of browsers, define the BROWSERS environment variable as a comma-seperated list.
// Supported browsers are 'ChromeHeadless', 'Safari', and 'Firefox'.
// See: https://karma-runner.github.io/6.4/config/browsers.html
browsers: determineBrowsers(),

webpack: webpackTestConfig,

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@
"karma-firefox-launcher": "2.1.3",
"karma-mocha": "2.0.1",
"karma-mocha-reporter": "2.2.5",
"karma-safari-launcher": "1.0.0",
"karma-sourcemap-loader": "0.4.0",
"karma-spec-reporter": "0.0.36",
"karma-summary-reporter": "3.1.1",
Expand Down Expand Up @@ -159,6 +158,7 @@
"undici": "6.19.7",
"watch": "1.0.2",
"webpack": "5.76.0",
"rimraf": "6.0.1",
"yargs": "17.7.2"
}
}
12 changes: 12 additions & 0 deletions packages/analytics/testing/integration-tests/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ async function checkForEventCalls(retryCount = 0): Promise<PerformanceEntry[]> {

describe('FirebaseAnalytics Integration Smoke Tests', () => {
let app: FirebaseApp;

// Clear cookies to prevent persistence issues across tests when run in non-headless browsers.
// This is required since gtag/logEvent behaviour is dependent on the session state; if an event has already been logged
// in a session, the event will be sent in the request payload instead of the query string.
beforeEach(() => {
document.cookie.split(';').forEach(cookie => {
document.cookie = cookie
.replace(/^ +/, '')
.replace(/=.*/, '=;expires=' + new Date().toUTCString() + ';path=/');
});
});

describe('Using getAnalytics()', () => {
afterEach(() => deleteApp(app));
it('logEvent() sends correct network request.', async () => {
Expand Down
9 changes: 0 additions & 9 deletions packages/auth-compat/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const files = ['src/**/*.test.ts'];

module.exports = function (config) {
const karmaConfig = Object.assign({}, karmaBase, {
browsers: getTestBrowsers(argv),
// files to load into karma
files: getTestFiles(),
preprocessors: { '**/*.ts': ['webpack', 'sourcemap'] },
Expand Down Expand Up @@ -56,14 +55,6 @@ function getTestFiles() {
}
}

function getTestBrowsers(argv) {
let browsers = ['ChromeHeadless'];
if (process.env?.BROWSERS && argv.unit) {
browsers = process.env?.BROWSERS?.split(',');
}
return browsers;
}

function getClientConfig() {
if (!argv.integration) {
return {};
Expand Down
11 changes: 1 addition & 10 deletions packages/auth/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const { argv } = require('yargs');

module.exports = function (config) {
const karmaConfig = Object.assign({}, karmaBase, {
browsers: getTestBrowsers(argv),
// files to load into karma
files: getTestFiles(argv),
// frameworks to use
Expand Down Expand Up @@ -60,7 +59,7 @@ function getTestFiles(argv) {
} else if (argv.cordova) {
return ['src/platform_cordova/**/*.test.ts'];
} else {
// For the catch-all yarn:test, ignore the phone integration test
// For the catch-all yarn browser:test, ignore the phone integration test
return [
'src/**/*.test.ts',
'test/helpers/**/*.test.ts',
Expand All @@ -71,14 +70,6 @@ function getTestFiles(argv) {
}
}

function getTestBrowsers(argv) {
let browsers = ['ChromeHeadless'];
if (process.env?.BROWSERS && argv.unit) {
browsers = process.env?.BROWSERS?.split(',');
}
return browsers;
}

function getClientConfig(argv) {
if (!argv.local) {
return {};
Expand Down
7 changes: 4 additions & 3 deletions packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,12 @@
"build:scripts": "tsc -moduleResolution node --module commonjs scripts/*.ts && ls scripts/*.js | xargs -I % sh -c 'terser % -o %'",
"dev": "rollup -c -w",
"test": "run-p --npm-path npm lint test:all",
"test:all": "run-p --npm-path npm test:browser:unit test:node:unit test:integration test:browser:integration:prodbackend",
"test:integration": "firebase emulators:exec --project emulatedproject --only auth \"run-s --npm-path npm test:browser:integration:local test:node:integration:local test:webdriver\"",
"test:all": "run-p --npm-path npm test:browser:unit test:node:unit test:integration",
"test:integration": "firebase emulators:exec --project emulatedproject --only auth \"run-s --npm-path npm test:browser:integration:local test:browser:integration:prodbackend test:node:integration:local test:webdriver\"",
"test:ci": "node ../../scripts/run_tests_in_ci.js -s test:all",
"test:integration:local": "run-s --npm-path npm test:node:integration:local test:browser:integration:local test:webdriver",
"test:browser": "karma start --single-run --local",
"test:browser": "karma start --single-run",
"test:browser:local": "karma start --single-run --local",
"test:browser:unit": "karma start --single-run --unit",
"test:browser:integration": "karma start --single-run --integration",
"test:browser:integration:local": "karma start --single-run --integration --local",
Expand Down
Loading
Loading