Skip to content

Commit

Permalink
Merge pull request qwikifiers#827 from cwoolum/main
Browse files Browse the repository at this point in the history
fix: get all popover tests running with polyfill
  • Loading branch information
thejackshelton authored Jun 6, 2024
2 parents 1101233 + bc14491 commit de0dfba
Show file tree
Hide file tree
Showing 18 changed files with 101 additions and 108 deletions.
5 changes: 5 additions & 0 deletions .changeset/twenty-spoons-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik-ui/headless': patch
---

Chromium 109-113 did not properly support the popover but reported that they did. This led to the polyfill not activating which caused our E2E tests to break. We are dropping down to Chromium 108 to validate the polyfill as users of Chrome would see it before the popover API was supported.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"test.headless": "nx component-test headless --skip-nx-cache",
"test.visual.headless": "nx visual-test headless",
"test.pw.headless": "nx e2e headless",
"test.pw.headless-chrome-113": "nx e2e-chrome-113 headless",
"test.pw.headless-chrome-108": "nx e2e-chrome-108 headless",
"test.pw.headless.ci": "nx e2e headless",
"test.headless.ci": "nx component-test-ci headless",
"test.utils": "nx test utils"
Expand Down
2 changes: 1 addition & 1 deletion packages/kit-headless/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"types": "./index.d.ts",
"type": "module",
"scripts": {
"setup.chrome.113": "npx tsx ./scripts/prepare-chrome-113.ts"
"setup.chrome.108": "npx tsx ./scripts/prepare-chrome-browser.ts"
},
"exports": {
".": {
Expand Down
10 changes: 5 additions & 5 deletions packages/kit-headless/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ let binaryPath;
switch (currentPlatform) {
case 'darwin':
binaryPath =
'./browsers/chrome/113/chrome-darwin/Chromium.app/Contents/MacOS/Chromium';
'./browsers/chrome/108/chrome-darwin/Chromium.app/Contents/MacOS/Chromium';
break;
case 'win32':
binaryPath = './browsers/chrome/113/chrome-win32/chrome.exe';
binaryPath = './browsers/chrome/108/chrome-win32/chrome.exe';
break;
case 'linux':
binaryPath = './browsers/chrome/113/chrome-linux/chrome';
binaryPath = './browsers/chrome/108/chrome-linux/chrome';
break;
default:
throw new Error('Cannot install Chrome 13 on unknown platform');
throw new Error('Cannot install Chrome 108 on unknown platform');
}

/**
Expand Down Expand Up @@ -65,7 +65,7 @@ export default defineConfig({
},

{
name: 'popover-chrome-113',
name: 'popover-chrome-108',
use: {
launchOptions: {
executablePath: path.resolve(dirName, binaryPath),
Expand Down
10 changes: 5 additions & 5 deletions packages/kit-headless/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,19 @@
"project": ["logic"]
}
},
"setup-chrome-113": {
"setup-chrome-108": {
"executor": "nx:run-script",
"options": {
"script": "setup.chrome.113"
"script": "setup.chrome.108"
}
},
"e2e-chrome-113": {
"e2e-chrome-108": {
"executor": "@nx/playwright:playwright",
"outputs": ["{workspaceRoot}/dist/.playwright/packages/kit-headless"],
"dependsOn": ["setup-chrome-113"],
"dependsOn": ["setup-chrome-108"],
"options": {
"config": "packages/kit-headless/playwright.config.ts",
"project": ["popover-chrome-113"]
"project": ["popover-chrome-108"]
}
},
"visual-test": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ function printProgress(progress: number) {
}

async function prepareChrome113() {
const path = `browsers/chrome/113/chrome-${os.platform()}`;
const path = `browsers/chrome/108/chrome-${os.platform()}`;

if (fs.existsSync(path)) {
console.log('Chrome 113 already exists. Skipping unzip ⏩');
console.log('Chrome 108 already exists. Skipping unzip ⏩');
} else {
console.log('Creating directory to save the browser 🌐');
fs.mkdirSync(path, { recursive: true });

const downloadResponse = await fetch(
`https://github.com/qwikifiers/qwik-ui-polyfill-browsers/raw/main/chrome/113/chrome-${os.platform()}.zip`,
`https://github.com/qwikifiers/qwik-ui-polyfill-browsers/raw/main/chrome/108/chrome-${os.platform()}.zip`,
);

const contentLength = downloadResponse.headers.get('content-length');
Expand All @@ -33,7 +33,7 @@ async function prepareChrome113() {
async start(controller) {
if (!downloadResponse.body) {
throw new Error(
'Invalid response received when trying to download Chrome 113',
'Invalid response received when trying to download Chrome 108',
);
}

Expand All @@ -53,7 +53,7 @@ async function prepareChrome113() {

await decompress(
Buffer.from(await res.arrayBuffer()),
`browsers/chrome/113/chrome-${os.platform()}`,
`browsers/chrome/108/chrome-${os.platform()}`,
{ strip: 1 },
);
}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ export const HPopoverTrigger = component$<PopoverTriggerProps>(
} = usePopover(context.compId);

const handleClick$ = $(async () => {
if (context.hover) return;
if (context.hover && !isSupportedSig.value) {
await showPopover();
return;
}

if (isSupportedSig.value) return;

Expand Down
13 changes: 12 additions & 1 deletion packages/kit-headless/src/components/popover/popover.driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,22 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
return rootLocator.locator('[popover]');
};

const getPopoverByTextContent = (popoverContent: string) => {
return rootLocator.locator('.popover-panel').getByText(popoverContent);
};

const getTrigger = () => {
return rootLocator.locator('[popovertarget]');
};

const openPopover = async (key: PopoverOpenKeys | 'click', index?: number) => {
const action = key === 'click' ? 'click' : 'press';
const trigger = index !== undefined ? getTrigger().nth(index) : getTrigger();
const popover = index !== undefined ? getPopover().nth(index) : getPopover();

const popover =
index !== undefined
? getPopoverByTextContent(`Popover ${index + 1}`)
: getPopover();

if (action === 'click') {
await trigger.click({ position: { x: 1, y: 1 } }); // Modified line
Expand All @@ -26,6 +34,8 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {

// Needed because Playwright doesn't wait for the listbox to be visible
await expect(popover).toBeVisible();

return { trigger, popover };
};

const getAllPopovers = () => {
Expand All @@ -49,5 +59,6 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
getAllTriggers,
openPopover,
getProgrammaticButtonTrigger,
getPopoverByTextContent,
};
}
Loading

0 comments on commit de0dfba

Please sign in to comment.