Skip to content

Add unit tests - #1993

Merged
alisa911 merged 7 commits into
mainfrom
080926_3
Sep 8, 2026
Merged

Add unit tests#1993
alisa911 merged 7 commits into
mainfrom
080926_3

Conversation

@alisa911

@alisa911 alisa911 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

AI disclaimer

Written with Claude (Cowork, Claude Fable 5.1), driven by @alisa911. Summary of the requests it worked from, in order:

  1. After a review of the previous PR found a test that pinned broken behaviour, asked to go through every unit test again and check whether each expectation is what the app should do, not what the code does. All 31 files were checked against the server (osmandapp/tools) and Android (osmandapp/android). One test was found to pin a real divergence - the web escapes both / and : in a favorite group file name while Android escapes only the first one it finds - and a few more doubtful cases; they went into KNOWN_ISSUES.md (not part of this PR), and the rule "expect what the app should do, not what it does" went into the add-test skill.

  2. Cover the routing cache of the track editor. effectControlRouterRequests let a seventh request through when six were in flight (> instead of >= against MAX_STARTED_ROUTER_JOBS); fixed after asking. Six tests in routing/01-routing-cache.test.js: a segment is cached by its points and profile, a failed request is not repeated, a routed segment goes into the track once, undo restores the geometry from the cache, the debouncer keeps the last call.

  3. Cover the distances of a track. getTrackPoints shifted distanceTotal of the second and following <trk> of a file in place, and it is called on every redraw of the graph, so the distance axis grew on each call. The author asked to re-check that the bug is real before fixing; the chain was verified end to end (WebGpxParser.getTracks returns one entry per <trk>, addDistance counts each from zero, getAllPoints returns references, GpxGraphProvider reads distanceTotal). Fixed by shifting copies; seven tests in tracks/14-track-distance.test.js. The first version of the test compared mutated points with themselves and passed - caught and rewritten to snapshot the numbers first.

  4. Cover deleting and reordering the points of a track. A point dragged to the first place in the Points tab kept the geometry of its old segment, so validateRoutePoints rejected the track and the analysis and the save ignored the reorder; fixed in insertPoint. The author reminded about the add-test skill: the inline contexts of the new tests were moved into fixtures/tracks.js (createEditorCtx, createEditorPoint, createRoutedPoints). Five tests in tracks/15-track-points.test.js.

  5. Cover the conversion of a navigation route into a track. No bugs; three tests in navigation/00-route-to-track.test.js (segments cut at the route points, a via point snapped to the line, alternatives left out, elevation borrowed at the ends of the line). Two doubts noted without a fix: a round trip whose finish is on the start collapses both into one map entry, and the point shared by two segments was counted twice in the average elevation - the latter was fixed on review, see 8.

  6. Cover the cloud changes and trash. deleteVersionsFromMenu kept the header of a month whose files were all deleted as long as any later month still had a file; the author asked to re-check, the rendering in CloudChanges / CloudTrash was confirmed to draw headers as they come; fixed. Five tests in settings/00-cloud-changes.test.js.

  7. Cover the url helpers, the file storages and the matched objects of the spatial search. No bugs; seven tests in util/07-url-and-storage.test.js and search/03-matched-objects.test.js. A matched amenity without coordinates would be listed as a link that does nothing - not pinned by a test, since the server builds matched objects from map objects with coordinates.

  8. Four review comments applied: deleteFile now goes through deleteVersionsFromMenu like restoreFile (the test had pinned the leftover month header again); isLast of the row that became the last of its month is recomputed, so no divider is drawn before the next header; the point shared by two segments of a route counts once in createAnalysisFromRoute (the test expects 115 with the arithmetic written down); the skill names the repositories instead of local paths.

No commits were made by the agent; every commit was reviewed and made by the author. The 406 unit tests were run by the agent, the selenium suite by the author.

Screenshot 2026-09-08 at 15 51 53

@alisa911

alisa911 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@alisa911
alisa911 requested a balanced review from Copilot September 8, 2026 12:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are well-covered by new unit tests and the production code edits are small, targeted, and consistent with the validated behaviors.

Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@alisa911
alisa911 requested a review from RZR-UA September 8, 2026 12:48
@alisa911
alisa911 merged commit 00afde7 into main Sep 8, 2026
1 check passed
@alisa911
alisa911 deleted the 080926_3 branch September 8, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants