Add QML tests for the reshape geometry editor tool#7584
Open
kaustuvpokharel wants to merge 9 commits into
Open
Add QML tests for the reshape geometry editor tool#7584kaustuvpokharel wants to merge 9 commits into
kaustuvpokharel wants to merge 9 commits into
Conversation
Collaborator
🍎 MacOS DMG universal buildsDownload a MacOS DMG universal build of this PR for testing. 📱 Android buildsDownload an Android arm64 build of this PR for testing. Other Android architectures🪟 Windows buildsDownload a Windows build of this PR for testing and for arm64. (Built from commit daa306e) 🐧 Linux AppImage buildsDownload a Linux AppImage build of this PR for testing. |
nirvn
reviewed
Jun 19, 2026
kaustuvpokharel
force-pushed
the
reshapeGeometryTests
branch
from
50f69b7 to
daa306e
Compare
nirvn
reviewed
Jun 26, 2026
Comment on lines
+86
to
+102
| function test_reshapeWithValidLineProducesExpectedGeometry() { | ||
| const model = initReshapeOnFields(); | ||
| // a reshape line that crosses the polygon boundary twice, cutting a new edge | ||
| rubberband.addVertexFromPoint(GeometryUtils.point(1030845.75, 5911397.39)); | ||
| rubberband.addVertexFromPoint(GeometryUtils.point(1030771.49, 5911511.09)); | ||
| rubberband.addVertexFromPoint(GeometryUtils.point(1030857.23, 5911624.79)); | ||
|
|
||
| // drive the operation directly and roll back instead of committing, so the | ||
| // shared layer data is untouched | ||
| if (!fieldsLayer.editBuffer()) | ||
| fieldsLayer.startEditing(); | ||
| const result = GeometryUtils.reshapeFromRubberband(fieldsLayer, model.feature.id, rubberband); | ||
| // the reshape succeeds and produces this exact polygon | ||
| compare(Number(result), Number(GeometryUtils.Success)); | ||
| const expected = "Polygon ((1031040.99 5911336.9, 1030978.97 5911394.33, 1030845.75 5911397.39, 1030845.75 5911397.4, 1030857.23 5911624.79, 1031057.07 5911646.23, 1031082.33 5911535.21, 1031119.08 5911493.1, 1031093.82 5911453.28, 1031072.67 5911421.57, 1031063.19 5911407.34, 1031044.82 5911362.94, 1031041.44 5911340.01, 1031040.99 5911336.9))"; | ||
| compare(fieldsLayer.getFeature(model.feature.id).geometry.asWkt(2), expected); | ||
| } |
Member
There was a problem hiding this comment.
We need to use the tool itself to insure it it sound (i.e. simulating a confirm click on the digitizing toolbar so the tool itself runs the onConfirmed function.
This test isn't necessarily bad (you are comparing a WKT, nice), but it's not testing what we want here: the tool itself.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This adds a QML test file for the reshape tool (Reshape.qml), and a small fix to the QML test harness that was needed to get there.
The harness fix first: mounting any component that contains a DigitizingToolbar currently crashes the QML test run with a segfault. The Cogo work added a CogoOperationsModel that dereferences CogoRegistry::instance() at construction, but that singleton is only set by QgisMobileapp, which the test harness never instantiates. So it comes back null and crashes. test_qml.cpp now sets a CogoRegistry instance in qmlEngineAvailable, the same way it already sets AppInterface. This unblocks testing any digitizing-based tool, not just reshape.
The tests themselves cover the tool wiring around the reshape maths (which is already covered in test_geometryutils.cpp): init sets up the feature model and forces polygon mode, blocking mirrors the rubberband digitizing state, the confirm handler's failure path toasts an error and rolls back leaving the feature untouched, and cancel resets the rubberband.
The successful reshape geometry is left to the C++ tests. Driving a valid reshape through confirm() here would mean reproducing the live digitizing rubberband state (the trailing current vertex, the cursor coordinate confirm() appends), which is awkward to do headless and adds little over the C++ coverage that already validates reshapeFromRubberband directly.
All tests pass locally.