Skip to content

fix: Updates test suite to correctly distinguish test types. #559

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ jobs:
uses: actions/cache@v4
with:
path: ~/.cache/ms-playwright # Default Playwright cache path
key: ${{ runner.os }}-playwright-${{ hashFiles('package-lock.json') }}-${{ hashFiles('playwright.config.ts') }}
key: ${{ runner.os }}-playwright-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('playwright.config.ts') }}
restore-keys: |
${{ runner.os }}-playwright-${{ hashFiles('package-lock.json') }}- # Fallback for lock file changes
${{ runner.os }}-playwright-${{ hashFiles('**/package-lock.json') }}- # Fallback for lock file changes
${{ runner.os }}-playwright- # Broader fallback

- name: Install dependencies
Expand Down Expand Up @@ -113,6 +113,7 @@ jobs:
env:
# Pass the correct base reference for find-changes.sh when called by samples.spec.ts
GIT_BASE_REF: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || github.event.before }}
TEST_SCOPE: 'affected'
CI: true

- name: Upload Test Report Artifact
Expand Down Expand Up @@ -148,9 +149,9 @@ jobs:
uses: actions/cache@v4
with:
path: ~/.cache/ms-playwright # Default Playwright cache path
key: ${{ runner.os }}-playwright-${{ hashFiles('package-lock.json') }}-${{ hashFiles('playwright.config.ts') }}
key: ${{ runner.os }}-playwright-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('playwright.config.ts') }}
restore-keys: |
${{ runner.os }}-playwright-${{ hashFiles('package-lock.json') }}- # Fallback for lock file changes
${{ runner.os }}-playwright-${{ hashFiles('**/package-lock.json') }}- # Fallback for lock file changes
${{ runner.os }}-playwright- # Broader fallback

- name: Install dependencies
Expand All @@ -164,10 +165,14 @@ jobs:

- name: Install Playwright browsers with OS dependencies
run: npx playwright install --with-deps
env:
DEBUG: pw:install # Enable Playwright's own install verbose logging
PLAYWRIGHT_BROWSERS_PATH: ~/.cache/ms-playwright # Explicitly set, though it's the default on Linux

- name: Run Full E2E Tests (Scheduled)
run: npx playwright test e2e/samples.spec.ts
env:
TEST_SCOPE: 'all'
CI: true

- name: Upload Test Report Artifact
Expand Down
215 changes: 115 additions & 100 deletions e2e/samples.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ const getAllSampleFolders = () => {
// Function to return only changed sample folders.
const getChangedSampleFolders = (): string[] => {
try {
const scriptPath = path.join(__dirname, '..', 'samples', 'find-changes.sh');
const scriptPath = path.join(__dirname, '..', 'samples', 'find-changes.sh');

if (!fs.existsSync(scriptPath)) {
console.warn(`Warning: find-changes.sh not found at ${scriptPath}. Running tests for all samples.`);
return getAllSampleFolders();
}

// Execute the script from the project root.
const projectRoot = path.join(__dirname, '..');
const projectRoot = path.join(__dirname, '..');
//const output = execSync(`sh ${scriptPath}`, { cwd: projectRoot, encoding: 'utf-8' });
const baseRefForScript = process.env.GIT_BASE_REF;
let commandToExecute = `bash ${scriptPath}`; // Use bash to ensure consistency with shebang
Expand All @@ -50,7 +50,7 @@ const getChangedSampleFolders = (): string[] => {
}
console.log(`Executing: ${commandToExecute}`);
const output = execSync(commandToExecute, { cwd: projectRoot, encoding: 'utf-8' });

const outputLines = output.trim().split('\n');
const changedFolders: string[] = [];
const markerLine = "Changed (added or modified) subfolders in 'samples/':";
Expand Down Expand Up @@ -95,25 +95,39 @@ const getChangedSampleFolders = (): string[] => {
}
};

const foldersToTest = getChangedSampleFolders();
let foldersToTest: string[];
const testScope = process.env.TEST_SCOPE;

if (foldersToTest.length === 0) {
console.log("No sample folders found.");
if (testScope === 'all') {
console.log('Collecting ALL samples for testing.');
foldersToTest = getAllSampleFolders();
} else if (testScope === 'affected') {
console.log('Collecting CHANGED samples for testing.');
foldersToTest = getChangedSampleFolders();
} else {
console.log(`Will run tests for the following folders: ${foldersToTest.join(', ')}`);
// Default behavior if TEST_SCOPE is not set or has an unexpected value
console.warn(`TEST_SCOPE is unset or has an unknown value ("${testScope}"). Defaulting to changed samples.`);
foldersToTest = getChangedSampleFolders();
}

// Iterate through samples and run the same test for each one.
foldersToTest.forEach((sampleFolder) => {
test(`test ${sampleFolder}`, async ({ page }) => {

// START Build the sample
const buildProcess = childProcess.spawn('npm', ['run', 'build'], {
cwd: path.join(samplesDir, sampleFolder),
stdio: 'inherit',
});
if (foldersToTest.length === 0) {
console.log("No sample folders found. Marking test suite as skipped.");
test.skip('No samples met the criteria for testing in this run', () => {
// This test will be marked as skipped and won't execute.
});
} else {
console.log(`Will run tests for the following folders: ${foldersToTest.join(', ')}`);
// Iterate through samples and run the same test for each one.
foldersToTest.forEach((sampleFolder) => {
test(`test ${sampleFolder}`, async ({ page }) => {

// START Build the sample
const buildProcess = childProcess.spawn('npm', ['run', 'build'], {
cwd: path.join(samplesDir, sampleFolder),
stdio: 'inherit',
});

await new Promise((resolve, reject) => {
await new Promise((resolve, reject) => {
buildProcess.on('close', (code) => {
if (code === 0) {
resolve(true);
Expand All @@ -122,99 +136,100 @@ foldersToTest.forEach((sampleFolder) => {
}
});
buildProcess.on('error', (err) => {
reject(new Error(`Failed to start build process for ${sampleFolder}: ${err.message}`));
reject(new Error(`Failed to start build process for ${sampleFolder}: ${err.message}`));
});
});
// END Build the sample

// START run the preview
// Get an available port
const port = 8080;
const url = `http://localhost:${port}/`;
// END Build the sample

const viteProcess = childProcess.spawn('vite', ['preview', `--port=${port}`], {
cwd: path.join(samplesDir, sampleFolder),
stdio: 'inherit',
detached: true, // Allows parent to exit independently, though we kill it in finally
});
// START run the preview
// Get an available port
const port = 8080;
const url = `http://localhost:${port}/`;

//await new Promise((resolve) => setTimeout(resolve, 500)); // Set a timeout to let the web server start.
await page.waitForTimeout(500);
// END run the preview

/**
* Run all of the tests. Each method call either runs a test or inserts a timeout for loading.
* `expect`s are assertions that test for conditions.
* Run `npx playwright test --ui` to launch Playwright in UI mode to iteratively debug this file.
*/
try {
const consoleErrors: string[] = [];
// Capture console errors and page errors
page.on('console', (msg) => {
if (msg.type() === 'error') {
consoleErrors.push(msg.text());
}
const viteProcess = childProcess.spawn('vite', ['preview', `--port=${port}`], {
cwd: path.join(samplesDir, sampleFolder),
stdio: 'inherit',
detached: true, // Allows parent to exit independently, though we kill it in finally
});
page.on('pageerror', (exception) => {
consoleErrors.push(exception.message);
});

// Navigate to the page.
//await page.goto(url, { waitUntil: 'networkidle', timeout: 500 });
await page.goto(url);

// Allow some time for async operations and errors to be caught
//await new Promise((resolve) => setTimeout(resolve, 500)); // Set a timeout to let the web server start.
await page.waitForTimeout(500);
// END run the preview

/**
* Run all of the tests. Each method call either runs a test or inserts a timeout for loading.
* `expect`s are assertions that test for conditions.
* Run `npx playwright test --ui` to launch Playwright in UI mode to iteratively debug this file.
*/
try {
const consoleErrors: string[] = [];
// Capture console errors and page errors
page.on('console', (msg) => {
if (msg.type() === 'error') {
consoleErrors.push(msg.text());
}
});
page.on('pageerror', (exception) => {
consoleErrors.push(exception.message);
});

// Filter out error messages we can safely avoid.
const filteredErrorMessages = [
'Falling back to Raster',
'Attempted to load a 3D Map, but failed.',
];
const criticalErrors = consoleErrors.filter(error =>
!filteredErrorMessages.some(message => error.includes(message))
);

if (criticalErrors.length > 0) {
console.error(`Critical console errors found in ${sampleFolder}:`, criticalErrors);
}
expect(criticalErrors).toHaveLength(0);
// Navigate to the page.
//await page.goto(url, { waitUntil: 'networkidle', timeout: 500 });
await page.goto(url);

// Wait for the page DOM to load; this does NOT include the Google Maps APIs.
await page.waitForLoadState('domcontentloaded', { timeout: 500 });
// Allow some time for async operations and errors to be caught
await page.waitForTimeout(500);

// Wait for Google Maps to load.
await page.waitForFunction(() => window.google && window.google.maps, { timeout: 500 });

// Insert a delay in ms to let the map load.
await page.waitForTimeout(500);
// Filter out error messages we can safely avoid.
const filteredErrorMessages = [
'Falling back to Raster',
'Attempted to load a 3D Map, but failed.',
];
const criticalErrors = consoleErrors.filter(error =>
!filteredErrorMessages.some(message => error.includes(message))
);

// Assertions. These must be met or the test will fail.
// The sample must load the Google Maps API.
const hasGoogleMaps = await page.evaluate(() => {
return typeof window.google !== 'undefined' && typeof window.google.maps !== 'undefined';
});
await expect(hasGoogleMaps).toBeTruthy();

/**const mapElement = await page.locator('#map');
if (await page.locator('#map').isVisible()) {
console.log(`✅ Assertion passed: Map is visible.`);
} else {
console.error(`❌ Assertion failed: Map is not visible.`);
throw new Error('Assertion failed: Map is not visible.');
}*/
} finally {
//viteProcess.kill(); // We used to just kill the process. Curious to see about how the other stuff works.
if (viteProcess.pid) {
try {
// Use process.kill for cross-platform compatibility, sending SIGINT
process.kill(viteProcess.pid, 'SIGINT');
} catch (e) {
console.warn(`Failed to kill Vite process for ${sampleFolder} (PID: ${viteProcess.pid}):`, e.message);
if (criticalErrors.length > 0) {
console.error(`Critical console errors found in ${sampleFolder}:`, criticalErrors);
}
expect(criticalErrors).toHaveLength(0);

// Wait for the page DOM to load; this does NOT include the Google Maps APIs.
await page.waitForLoadState('domcontentloaded', { timeout: 500 });

// Wait for Google Maps to load.
await page.waitForFunction(() => window.google && window.google.maps, { timeout: 500 });

// Insert a delay in ms to let the map load.
await page.waitForTimeout(500);

// Assertions. These must be met or the test will fail.
// The sample must load the Google Maps API.
const hasGoogleMaps = await page.evaluate(() => {
return typeof window.google !== 'undefined' && typeof window.google.maps !== 'undefined';
});
await expect(hasGoogleMaps).toBeTruthy();

/**const mapElement = await page.locator('#map');
if (await page.locator('#map').isVisible()) {
console.log(`✅ Assertion passed: Map is visible.`);
} else {
console.error(`❌ Assertion failed: Map is not visible.`);
throw new Error('Assertion failed: Map is not visible.');
}*/
} finally {
//viteProcess.kill(); // We used to just kill the process. Curious to see about how the other stuff works.
if (viteProcess.pid) {
try {
// Use process.kill for cross-platform compatibility, sending SIGINT
process.kill(viteProcess.pid, 'SIGINT');
} catch (e) {
console.warn(`Failed to kill Vite process for ${sampleFolder} (PID: ${viteProcess.pid}):`, e.message);
}
}
// Add a small delay to allow the process to terminate
await page.waitForTimeout(500);
}
// Add a small delay to allow the process to terminate
await page.waitForTimeout(500);
}
});
});
});
}
2 changes: 1 addition & 1 deletion samples/test-example/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Copyright 2025 Google LLC. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
// TEST COMMENT 003
// TEST COMMENT 004
// [START maps_test_example]
// Initialize and add the map.
let map;
Expand Down