-
Notifications
You must be signed in to change notification settings - Fork 20
chore: automated tests for select one for map #509
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
Merged
Merged
Changes from all commits
Commits
Show all changes
93 commits
Select commit
Hold shift + click to select a range
517792e
Merge branch 'main' of https://github.com/getodk/web-forms into selec…
latin-panda ef0cb3a
Merge branch 'main' of https://github.com/getodk/web-forms into selec…
latin-panda 0cd76eb
engine support for metadata in itemset nodes
latin-panda 68da647
Merge branch 'main' of https://github.com/getodk/web-forms into selec…
latin-panda 16f3cde
Async map bundle loading
latin-panda 66e9764
typing
latin-panda a7dea73
Map Component
latin-panda 4823ebf
Adds new icons
latin-panda 1e9feec
removes cache buste and timeout
latin-panda b009dc7
Map UI features: properties popup and status bar
latin-panda 93cc3a8
Merge branch 'main' of https://github.com/getodk/web-forms into selec…
latin-panda 525d890
Merge branch 'select-one-for-map' of https://github.com/getodk/web-fo…
latin-panda 01a4fa5
fixes styles and icons based on feedback
latin-panda 78872b2
Merge branch 'main' of https://github.com/getodk/web-forms into ui-se…
latin-panda dcf7902
Merge branch 'main' of https://github.com/getodk/web-forms into ui-se…
latin-panda 313e751
Reverts changes in engine. We decided another solution
latin-panda 6c0ab6f
Applies latest designer feedback
latin-panda 4b0a7e4
fixes lint, handles esc key
latin-panda 2467720
Makes lines less thick
latin-panda 294ddd5
stacking errors
latin-panda a4d3056
Adds map background for full screen
latin-panda 087645f
Adds support for properties in the engine
latin-panda 34ebdb4
Fixes scenario tests
latin-panda 62562bf
Integration engine and client
latin-panda 7ff1896
Refactored component input types and fixed view center when no features
latin-panda f1494a6
Simplified map ref and source get
latin-panda c7657c2
Flat and prefixed properties to avoid OL conflicts, fixes polygon cal…
latin-panda 07d288d
fixes click binding for disable question
latin-panda 40b9ea4
fixes lint
latin-panda d944129
Feedback for async map and properties components
latin-panda 8b9db70
Feedback for useMapBlock
latin-panda 4ec6fbc
Feedback for line's hit tolerance
latin-panda 6ecc1a0
Feedback for line's hit tolerance
latin-panda 84a54a5
Merge branch 'main' of https://github.com/getodk/web-forms into ui-se…
latin-panda acb419f
changeset
latin-panda 0705a5d
Improves style filters
latin-panda 979ec9d
Feedback - properties popup
latin-panda 61602e3
Feedback - map buttons
latin-panda 99305c0
Remove dummy code
latin-panda 6c3ce11
feedback - remove cursor when map is disabled
latin-panda 8d6b536
feedback - removes watch options, use better function to find features
latin-panda c669258
feedback - uses shallowRef
latin-panda 11d81ea
feedback - Adds animation to feature center!
latin-panda eec9229
Feedback - Vertical alignment of popup lines and add zoom to fit all …
latin-panda 40652ac
Fix to match Collect error handling
latin-panda 9992b63
Removes Google Chrome's tap highlight
latin-panda 6cf13f1
QA feedback - Prevent map cloning at low zoom
latin-panda 8d01be7
Initial e2e tests for select one from map
latin-panda 956a35d
scope selectors to specific question
latin-panda 567d170
snapshots for firefox and webkit
latin-panda 942bce5
separate suite for permissions related tests
latin-panda 18a3ccd
snapshots
latin-panda 88ab353
simplify map test and add lines and polygons
latin-panda a7be09f
remove unnecessary awaits
latin-panda ae870f3
screenshots for visual tests
latin-panda 519b534
screenshots for visual tests
latin-panda 70a8977
fixes lint
latin-panda d5cd51f
Take-1: try giving more time to map rendering
latin-panda b412094
Removes checks that are covered by the snapshots
latin-panda ab236b9
Removes timeouts
latin-panda 3c80073
button match
latin-panda ba5c0dc
Makes snapshot comparison more sensitive
latin-panda 52242c7
artifacts for debugging
latin-panda 7006c19
fixes snapshots
latin-panda 3b23c44
take-2: turns off headless
latin-panda 75b0670
take-3: turns off headless and uses xvfb-run for display
latin-panda 1ba994e
take-4: config in xvfb-run for consistent rendering
latin-panda 327ae6b
take-5: try scale css
latin-panda 7e5e872
take-6: clip
latin-panda e37887c
take-7: clip and match function
latin-panda 47775b8
take-7-2: DEBUG
latin-panda 100b7ce
take-7-2: DEBUG
latin-panda 26b4a68
take-7-3: DEBUG
latin-panda c3ba116
take-7-4: DEBUG, set styles
latin-panda 12e598b
take-7-4: DEBUG, set styles
latin-panda b064074
take-8: browser rendering settings
latin-panda e173313
take-8: browser rendering settings
latin-panda 9cfe196
take-8: styling fix
latin-panda 32f01bd
take-8: revert style changes
latin-panda 29380fe
take-8: revert style changes
latin-panda 93e87cb
take-8: fullscreen adjustment
latin-panda 89e42b1
add back permissions
latin-panda 920d995
add back permissions
latin-panda 6b30bf5
Testing minimum args needed - Take 1
latin-panda b9ed806
Cleaning
latin-panda 0cf1f23
more cleaning
latin-panda 4d838d5
test: Can we do less css changes?
latin-panda 1466ee6
test-2: Can we do less css changes?
latin-panda 3fc2c6b
test-3: Can we do less css changes?
latin-panda 5149154
test-4: Can we do less css changes?
latin-panda 719525d
unit test for conversion of ODK features to GeoJSON
latin-panda 0629351
fixes format
latin-panda ce3045b
Merge branch 'main' of https://github.com/getodk/web-forms into e2e-s…
latin-panda File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
183 changes: 183 additions & 0 deletions
183
packages/web-forms/e2e/page-objects/controls/MapControl.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
import { expect, Locator, Page } from '@playwright/test'; | ||
|
||
export class MapControl { | ||
private readonly MAP_COMPONENT_SELECTOR = '.map-block-component'; | ||
private readonly MAP_CONTAINER_SELECTOR = '.map-container'; | ||
private readonly MAP_SELECTOR = '.map-block'; | ||
private readonly ANIMATION_TIME = 1000; // Map has a default of 1s rendering and animation time | ||
|
||
private readonly page: Page; | ||
|
||
constructor(page: Page) { | ||
this.page = page; | ||
} | ||
|
||
getMapComponentLocator(label: string) { | ||
const question = this.page | ||
.locator('.question-container') | ||
.filter({ has: this.page.locator(`.control-text label span:text-is("${label}")`) }); | ||
return question.locator(this.MAP_COMPONENT_SELECTOR); | ||
} | ||
|
||
async expectMapVisible(mapComponent: Locator) { | ||
const map = mapComponent.locator(this.MAP_SELECTOR); | ||
await expect(map, `Map not found`).toBeVisible(); | ||
} | ||
|
||
/** | ||
* Playwright's scrollIntoViewIfNeeded() scrolls minimally and doesn't guarantee to center | ||
* the element, so JavaScript scroll is used to ensure the map is centered and fully visible. | ||
*/ | ||
async scrollMapIntoView(mapComponent: Locator) { | ||
const handle = await mapComponent.elementHandle(); | ||
if (handle) { | ||
await handle.evaluate((el) => el.scrollIntoView({ block: 'center' })); | ||
} | ||
} | ||
|
||
/** | ||
* Pans the map starting from the center of the component. | ||
* @param moveX Pixels. Relative to the map component center. | ||
* @param moveY Pixels. Relative to the map component center. | ||
*/ | ||
async panMap(mapComponent: Locator, moveX: number, moveY: number) { | ||
await mapComponent.scrollIntoViewIfNeeded(); | ||
const mapContainer = mapComponent.locator(this.MAP_CONTAINER_SELECTOR); | ||
const box = await mapContainer.boundingBox(); | ||
if (!box) { | ||
return; | ||
} | ||
|
||
const centerX = box.x + box.width / 2; | ||
const centerY = box.y + box.height / 2; | ||
|
||
await this.page.mouse.move(centerX, centerY); | ||
await this.page.mouse.down(); | ||
await this.page.mouse.move(centerX + moveX, centerY + moveY, { steps: 40 }); | ||
|
||
await this.page.mouse.up(); | ||
} | ||
|
||
async zoomIn(mapComponent: Locator, times = 1) { | ||
const button = mapComponent.locator('.ol-zoom').getByText('+', { exact: true }); | ||
await expect(button).toBeVisible(); | ||
for (let i = 0; i < times; i++) { | ||
await button.click(); | ||
// Allows reaching the expected zoom level before zooming again for consistent results. | ||
await this.page.waitForTimeout(this.ANIMATION_TIME); | ||
} | ||
} | ||
|
||
async zoomOut(mapComponent: Locator, times = 1) { | ||
const button = mapComponent.locator('.ol-zoom').getByText('–', { exact: true }); | ||
await expect(button).toBeVisible(); | ||
for (let i = 0; i < times; i++) { | ||
await button.click(); | ||
// Allows reaching the expected zoom level before zooming again for consistent results. | ||
await this.page.waitForTimeout(this.ANIMATION_TIME); | ||
} | ||
} | ||
|
||
async zoomToFitAll(mapComponent: Locator) { | ||
const button = mapComponent.locator('button[title="Zoom to fit all options"]'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
} | ||
|
||
async centerCurrentLocation(mapComponent: Locator) { | ||
const button = mapComponent.locator('button[title="Zoom to current location"]'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
} | ||
|
||
async toggleFullScreen(mapComponent: Locator) { | ||
const button = mapComponent.locator('button[title="Full Screen"]'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
} | ||
|
||
async expectFullScreenActive(mapComponent: Locator) { | ||
await expect(mapComponent).toHaveAttribute('class', expect.stringContaining('map-full-screen')); | ||
} | ||
|
||
async exitFullScreen(mapComponent: Locator) { | ||
await this.page.keyboard.press('Escape'); | ||
await expect(mapComponent.locator('.map-full-screen')).not.toBeVisible(); | ||
} | ||
|
||
/** | ||
* Selects feature on map | ||
* @param positionX Pixels. Relative to the left of the browser's viewport. | ||
* @param positionY Pixels. Relative to the top of the browser's viewport. | ||
*/ | ||
async selectFeature(positionX: number, positionY: number) { | ||
await this.page.mouse.move(positionX, positionY); | ||
await this.page.mouse.down(); | ||
await this.page.mouse.up(); | ||
} | ||
|
||
async saveSelection(mapComponent: Locator) { | ||
const button = mapComponent.locator('.map-properties').getByText('Save selected'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
} | ||
|
||
async removeSavedFeature(mapComponent: Locator) { | ||
const button = mapComponent.locator('.map-properties').getByText('Remove selection'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
} | ||
|
||
async viewDetailsOfSavedFeature(mapComponent: Locator) { | ||
const button = mapComponent.locator('.map-status-saved').getByText('View details'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
} | ||
|
||
async closeProperties(mapComponent: Locator) { | ||
const button = mapComponent.locator('.map-properties .close-icon'); | ||
await expect(button).toBeVisible(); | ||
await button.click(); | ||
await expect(mapComponent.locator('.map-properties')).not.toBeVisible(); | ||
} | ||
|
||
async expectErrorMessage(mapComponent: Locator, expectedTitle: string, expectedMessage: string) { | ||
const error = mapComponent.locator('.map-block-error'); | ||
await expect(error.locator('strong')).toHaveText(expectedTitle); | ||
await expect(error.locator('span')).toHaveText(expectedMessage); | ||
} | ||
|
||
async expectMapScreenshot(mapComponent: Locator, snapshotName: string, isFullScreen = false) { | ||
// It cannot disable map's JS animations. So setting timeout. | ||
await this.page.waitForTimeout(this.ANIMATION_TIME); | ||
|
||
const map = mapComponent.locator(this.MAP_CONTAINER_SELECTOR); | ||
const browserName = this.page.context().browser()?.browserType().name(); | ||
const isChromiumLinux = process.platform === 'linux' && browserName === 'chromium'; | ||
|
||
if (isChromiumLinux) { | ||
// Chrome for Linux has an issue when taking the snapshot (ref. https://github.com/microsoft/playwright/issues/18827) | ||
const width = isFullScreen ? 1280 : 802; | ||
const heigth = isFullScreen ? 720 : 507; | ||
await this.page.addStyleTag({ | ||
content: ` | ||
.map-container { | ||
width: ${width}px !important; | ||
height: ${heigth}px !important; | ||
max-width: ${width}px !important; | ||
max-height: ${heigth + 1}px !important; | ||
} | ||
body, html { | ||
overflow: hidden !important; | ||
} | ||
`, | ||
}); | ||
|
||
await this.page.waitForTimeout(this.ANIMATION_TIME); | ||
} | ||
|
||
await expect(map).toHaveScreenshot(snapshotName, { | ||
maxDiffPixels: 5000, | ||
}); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,36 @@ | ||
import { Page } from '@playwright/test'; | ||
import { GeopointControl } from '../controls/GeopointControl.js'; | ||
import { InputControl } from '../controls/InputControl.js'; | ||
import { MapControl } from '../controls/MapControl.js'; | ||
import { RepeatControl } from '../controls/RepeatControl.js'; | ||
import { TextControl } from '../controls/TextControl.js'; | ||
|
||
export class FillFormPage { | ||
private readonly page: Page; | ||
|
||
public readonly geopoint: GeopointControl; | ||
public readonly input: InputControl; | ||
public readonly map: MapControl; | ||
public readonly repeat: RepeatControl; | ||
public readonly text: TextControl; | ||
public readonly input: InputControl; | ||
|
||
constructor(page: Page) { | ||
this.page = page; | ||
|
||
this.geopoint = new GeopointControl(page); | ||
this.input = new InputControl(page); | ||
this.map = new MapControl(page); | ||
this.repeat = new RepeatControl(page); | ||
this.text = new TextControl(page); | ||
this.input = new InputControl(page); | ||
} | ||
|
||
async copyToClipboard(valueToCopy: string) { | ||
await this.page.evaluate((value) => { | ||
return navigator.clipboard.writeText(value); | ||
}, valueToCopy); | ||
} | ||
|
||
async waitForNetworkIdle() { | ||
return this.page.waitForLoadState('networkidle'); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This number sounds huge until you realise the image is huge. I'm still surprised that the variation can be this big. Did you have a look at the diffs to figure out what is changing?
I think it'd make me feel better if it was a ratio, eg
This is roughly equivalent but if we got a smaller image (eg: mobile resolution tests) then the error threshold would scale down too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I compared the diff, and the large difference mainly comes from the map. For example, if the zoom level ends up being off by just a couple of pixels, the map lines and tiles shift slightly, which makes the screenshot comparison detect a big pixel difference, even though the UI looks the same to humans 😅
That’s why I moved the test points from Europe to South Australia, that area has fewer visual map details (just a few roads and a small town), so there’s less variation between runs.
For other question types (not map-related), I’d expect the pixel difference to be much smaller, around 100 pixels or so.
I also tried adjusting
maxDiffPixelRatio
with small values like0.0XX
, but it only seemed to work with values like0.1
,0.2
, etc.