Skip to content

Conversation

@reakaleek
Copy link
Member

What

Added ImagePathResolutionTests.cs to verify DiagnosticLinkInlineParser.UpdateRelativeUrl correctly resolves image paths in both assembler and non-assembler builds.
Why: In assembler builds, documentation from multiple repos is combined with each section getting a path_prefix from navigation.yml (e.g., platform, reference/elasticsearch). Image URLs must include this prefix: images/pic.png/docs/platform/setup/images/pic.png. Non-assembler builds skip the prefix: /docs/setup/images/pic.png.

How it works

Mock a minimal doc set with a markdown file at setup/guide.md referencing images/pic.png
Seed navigation metadata by injecting a stub INavigationItem into MarkdownNavigationLookup with the expected URL (e.g., /platform/setup/guide).. this mimics what DocumentationSetNavigation produces in real assembler builds
Call UpdateRelativeUrl which reads the stub's URL, extracts the directory path, and resolves the image relative to it
Assert the result includes (or excludes) the path_prefix based on AssemblerBuild flag

Why

By stubbing navigation metadata instead of building the full navigation tree, the test stays fast, isolated, and focused on UpdateRelativeUrl behavior. The stub recreates the exact state the production code sees.

@reakaleek reakaleek requested a review from a team as a code owner November 13, 2025 09:36
@reakaleek reakaleek requested a review from cotti November 13, 2025 09:36
@reakaleek reakaleek mentioned this pull request Nov 13, 2025
@reakaleek reakaleek requested a review from Mpdreamz November 13, 2025 09:41
Copy link
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

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

LGTM, Another option would be to extend our integration tests since they inject docs-builder docs in the navigation we can include some more tests (referencing images from snippets comes to mind).

I might follow up with that actually.

Copy link
Contributor

@cotti cotti left a comment

Choose a reason for hiding this comment

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

LGTM, I was implementing this by extending InlineTest to have an assembler-mode option, but this is better.

@cotti cotti merged commit 945d578 into fix/image_paths Nov 13, 2025
21 of 22 checks passed
@cotti cotti deleted the feature/add-image-path-tests branch November 13, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants