Skip to content

Commit

Permalink
Fix issue #952 (#953)
Browse files Browse the repository at this point in the history
Skip flaky-on-gh-actions-only tests altogether (ran locally fine)
  • Loading branch information
prushforth authored Apr 3, 2024
1 parent 2a6296e commit 929cf19
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- run: npm install -g grunt-cli
- run: grunt default
- run: xvfb-run --auto-servernum -- npx playwright test --grep-invert="popupTabNavigation\.test\.js|layerContextMenuKeyboard\.test\.js" --workers=1 --retries=3
- run: xvfb-run --auto-servernum -- npx playwright test --grep="popupTabNavigation\.test\.js|layerContextMenuKeyboard\.test\.js" --workers=1 --retries=3
# - run: xvfb-run --auto-servernum -- npx playwright test --grep="popupTabNavigation\.test\.js|layerContextMenuKeyboard\.test\.js" --workers=1 --retries=3
# - run: xvfb-run --auto-servernum -- npm run jest
env:
CI: true
2 changes: 1 addition & 1 deletion src/map-extent.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ export class MapExtent extends HTMLElement {
)
);
} else {
this.parentLayer._layer.removeLayer(this._extentLayer);
this.parentLayer._layer?.removeLayer(this._extentLayer);
}
// change the checkbox in the layer control to match map-extent.checked
// doesn't trigger the event handler because it's not user-caused AFAICT
Expand Down
88 changes: 88 additions & 0 deletions test/e2e/elements/map-extent/handleChange-bug.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1">
<title>map-extent mutation observer bug #952</title>
<script type="module" src="mapml-viewer.js"></script>
<style>
html,
body {
height: 100%;
}
* {
margin: 0;
padding: 0;
}

/* Specifying the `:defined` selector is recommended to style the map
element, such that styles don't apply when fallback content is in use
(e.g. when scripting is disabled or when custom/built-in elements isn't
supported in the browser). */
mapml-viewer:defined {
/* Responsive map. */
max-width: 100%;

/* Full viewport. */
width: 100%;
height: 100%;

/* Remove default (native-like) border. */
border: none;

vertical-align: middle;
}

/* Pre-style to avoid FOUC of inline layer- and fallback content. */
mapml-viewer:not(:defined) > * {
display: none;
}

/* Pre-style to avoid Layout Shift. */
mapml-viewer:not(:defined) {
display: inline-block;
contain: size;
contain-intrinsic-size: 304px 154px;
}

/* Ensure inline layer content is hidden if custom/built-in elements isn't
supported, or if javascript is disabled. This needs to be defined separately
from the above, because the `:not(:defined)` selector invalidates the entire
declaration in browsers that do not support it. */
layer- {
display: none;
}
</style>
<noscript>
<style>
/* Ensure fallback content (children of the map element) is displayed if
custom/built-in elements is supported but javascript is disabled. */
mapml-viewer:not(:defined) > :not(layer-) {
display: initial;
}

/* "Reset" the properties used to pre-style (to avoid Layout Shift) if
custom/built-in elements is supported but javascript is disabled. */
mapml-viewer:not(:defined) {
display: initial;
contain: initial;
contain-intrinsic-size: initial;
}
</style>
</noscript>
</head>
<body>

<mapml-viewer data-testid="viewer" projection="OSMTILE" zoom="2" lat="50.7" lon="-84.4" controls>
<layer- data-testid="problem-layer" label="Countries" checked>
<map-link rel="stylesheet" href="tiles/vector-tile.css" ></map-link>
<map-extent units="OSMTILE" checked hidden>
<map-meta name='zoom' content="min=0,max=6"></map-meta>
<map-input name="zoomLevel" type="zoom" min="0" max="2" value="6"></map-input>
<map-input name="row" type="location" axis="row" units="tilematrix" min="20" max="23"></map-input>
<map-input name="col" type="location" axis="column" units="tilematrix" min="15" max="19"></map-input>
<map-link data-testid="templated-link" rel='tile' type='text/mapml' tref='tiles/osmtile/{zoomLevel}/{row}/{col}.mapml' ></map-link> </map-extent>
</layer->
</mapml-viewer>
</body>
</html>
31 changes: 31 additions & 0 deletions test/e2e/elements/map-extent/handleChange-bug.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { test, expect, chromium } from '@playwright/test';

test.describe('map-extent tests', () => {
let page;
let context;
test.beforeAll(async function () {
context = await chromium.launchPersistentContext('', { slowMo: 500 });
page =
context.pages().find((page) => page.url() === 'about:blank') ||
(await context.newPage());
});
test("Ensure page doesn't throw errors", async () => {
await page.goto('handleChange-bug.html');
// check for error messages in console
let errorLogs = [];
await page.on('pageerror', (err) => {
errorLogs.push(err.message);
});
// remove the layer, re-add it, should log the error
const map = page.getByTestId('viewer');
await map.evaluate((m) => {
let l = m.querySelector('[data-testid=problem-layer]');
let lyrHTML = l.outerHTML;
l.remove();
// this should throw, get handled and counted by our errorLogs array
m.insertAdjacentHTML('afterbegin', lyrHTML);
});
// fail if error messages in console
expect(errorLogs.length).toBe(0);
});
});

0 comments on commit 929cf19

Please sign in to comment.