Skip to content

Commit efcebd7

Browse files
authored
Merge pull request #40630 from github/repo-sync
Repo sync
2 parents 41ddffe + 556af50 commit efcebd7

File tree

20 files changed

+668
-67
lines changed

20 files changed

+668
-67
lines changed

src/content-linter/lib/linting-rules/frontmatter-landing-recommended.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,23 @@ import { addError } from 'markdownlint-rule-helpers'
55
import { getFrontmatter } from '../helpers/utils'
66

77
function isValidArticlePath(articlePath, currentFilePath) {
8-
// Article paths in recommended are relative to the current page's directory
9-
const relativePath = articlePath.startsWith('/') ? articlePath.substring(1) : articlePath
8+
const ROOT = process.env.ROOT || '.'
9+
10+
// Strategy 1: Always try as an absolute path from content root first
11+
const contentDir = path.join(ROOT, 'content')
12+
const normalizedPath = articlePath.startsWith('/') ? articlePath.substring(1) : articlePath
13+
const absolutePath = path.join(contentDir, `${normalizedPath}.md`)
14+
15+
if (fs.existsSync(absolutePath) && fs.statSync(absolutePath).isFile()) {
16+
return true
17+
}
18+
19+
// Strategy 2: Fall back to relative path from current file's directory
1020
const currentDir = path.dirname(currentFilePath)
11-
const fullPath = path.join(currentDir, `${relativePath}.md`)
21+
const relativePath = path.join(currentDir, `${normalizedPath}.md`)
1222

1323
try {
14-
return fs.existsSync(fullPath) && fs.statSync(fullPath).isFile()
24+
return fs.existsSync(relativePath) && fs.statSync(relativePath).isFile()
1525
} catch {
1626
return false
1727
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
title: Test Absolute Only Path
3+
layout: product-landing
4+
versions:
5+
fpt: '*'
6+
recommended:
7+
- /article-two
8+
---
9+
10+
# Test Absolute Only Path
11+
12+
This tests /article-two which exists in content/article-two.md but NOT in the current directory.
13+
If relative paths were tried first, this would fail.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
title: Test Absolute Path Priority
3+
layout: product-landing
4+
versions:
5+
fpt: '*'
6+
recommended:
7+
- /article-one
8+
- /subdir/article-three
9+
---
10+
11+
# Test Absolute Path Priority
12+
13+
Testing that absolute paths are prioritized over relative paths.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
title: Test Path Priority Resolution
3+
layout: product-landing
4+
versions:
5+
fpt: '*'
6+
recommended:
7+
- /article-one
8+
---
9+
10+
# Test Path Priority Resolution
11+
12+
This tests that /article-one resolves to the absolute path:
13+
tests/fixtures/fixtures/content/article-one.md (absolute from fixtures root)
14+
NOT the relative path:
15+
tests/fixtures/landing-recommended/article-one.md (relative to this file)
16+
17+
The absolute path should be prioritized over the relative path.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
title: Test Priority Validation
3+
layout: product-landing
4+
versions:
5+
fpt: '*'
6+
recommended:
7+
- /article-one
8+
- /nonexistent-absolute
9+
---
10+
11+
# Test Priority Validation
12+
13+
This tests that /article-one resolves correctly AND that /nonexistent-absolute fails properly.
14+
The first should pass (exists in absolute path), the second should fail (doesn't exist anywhere).

src/content-linter/tests/unit/frontmatter-landing-recommended.js

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, test } from 'vitest'
1+
import { describe, expect, test, beforeAll, afterAll } from 'vitest'
22

33
import { runRule } from '@/content-linter/lib/init-test'
44
import { frontmatterLandingRecommended } from '@/content-linter/lib/linting-rules/frontmatter-landing-recommended'
@@ -10,13 +10,28 @@ const DUPLICATE_RECOMMENDED =
1010
'src/content-linter/tests/fixtures/landing-recommended/duplicate-recommended.md'
1111
const INVALID_PATHS = 'src/content-linter/tests/fixtures/landing-recommended/invalid-paths.md'
1212
const NO_RECOMMENDED = 'src/content-linter/tests/fixtures/landing-recommended/no-recommended.md'
13+
const ABSOLUTE_PRIORITY =
14+
'src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md'
15+
const PATH_PRIORITY = 'src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md'
16+
const ABSOLUTE_ONLY = 'src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md'
17+
const PRIORITY_VALIDATION =
18+
'src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md'
1319

1420
const ruleName = frontmatterLandingRecommended.names[1]
1521

1622
// Configure the test fixture to not split frontmatter and content
1723
const fmOptions = { markdownlintOptions: { frontMatter: null } }
1824

1925
describe(ruleName, () => {
26+
const envVarValueBefore = process.env.ROOT
27+
28+
beforeAll(() => {
29+
process.env.ROOT = 'src/fixtures/fixtures'
30+
})
31+
32+
afterAll(() => {
33+
process.env.ROOT = envVarValueBefore
34+
})
2035
test('landing page with recommended articles passes', async () => {
2136
const result = await runRule(frontmatterLandingRecommended, {
2237
files: [VALID_LANDING],
@@ -75,4 +90,61 @@ describe(ruleName, () => {
7590
})
7691
expect(result[VALID_LANDING]).toEqual([])
7792
})
93+
94+
test('absolute paths are prioritized over relative paths', async () => {
95+
// This test verifies that when both absolute and relative paths exist with the same name,
96+
// the absolute path is chosen over the relative path.
97+
//
98+
// Setup:
99+
// - /article-one should resolve to src/fixtures/fixtures/content/article-one.md (absolute)
100+
// - article-one (relative) would resolve to src/content-linter/tests/fixtures/landing-recommended/article-one.md
101+
//
102+
// The test passes because our logic prioritizes the absolute path resolution first
103+
const result = await runRule(frontmatterLandingRecommended, {
104+
files: [ABSOLUTE_PRIORITY],
105+
...fmOptions,
106+
})
107+
expect(result[ABSOLUTE_PRIORITY]).toEqual([])
108+
})
109+
110+
test('path priority resolution works correctly', async () => {
111+
// This test verifies that absolute paths are prioritized over relative paths
112+
// when both files exist with the same name.
113+
//
114+
// Setup:
115+
// - /article-one could resolve to EITHER:
116+
// 1. src/fixtures/fixtures/content/article-one.md (absolute - should be chosen)
117+
// 2. src/content-linter/tests/fixtures/landing-recommended/article-one.md (relative - should be ignored)
118+
//
119+
// Our prioritization logic should choose #1 (absolute) over #2 (relative)
120+
// This test passes because the absolute path exists and is found first
121+
const result = await runRule(frontmatterLandingRecommended, {
122+
files: [PATH_PRIORITY],
123+
...fmOptions,
124+
})
125+
expect(result[PATH_PRIORITY]).toEqual([])
126+
})
127+
128+
test('absolute-only paths work when no relative path exists', async () => {
129+
// This test verifies that absolute path resolution works when no relative path exists
130+
// /article-two exists in src/fixtures/fixtures/content/article-two.md
131+
// but NOT in src/content-linter/tests/fixtures/landing-recommended/article-two.md
132+
// This test would fail if we didn't prioritize absolute paths properly
133+
const result = await runRule(frontmatterLandingRecommended, {
134+
files: [ABSOLUTE_ONLY],
135+
...fmOptions,
136+
})
137+
expect(result[ABSOLUTE_ONLY]).toEqual([])
138+
})
139+
140+
test('mixed valid and invalid absolute paths are handled correctly', async () => {
141+
// This test has both a valid absolute path (/article-one) and an invalid one (/nonexistent-absolute)
142+
// It should fail because of the invalid path, proving our absolute path resolution is working
143+
const result = await runRule(frontmatterLandingRecommended, {
144+
files: [PRIORITY_VALIDATION],
145+
...fmOptions,
146+
})
147+
expect(result[PRIORITY_VALIDATION]).toHaveLength(1)
148+
expect(result[PRIORITY_VALIDATION][0].errorDetail).toContain('nonexistent-absolute')
149+
})
78150
})
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
title: Article One (ABSOLUTE VERSION - Should be used)
3+
---
4+
5+
# Article One (Absolute Version)
6+
7+
This is the ABSOLUTE version in the fixtures root directory.
8+
If this file is being resolved, the prioritization is CORRECT!
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
title: Article One (ABSOLUTE VERSION - Should be used)
3+
---
4+
5+
# Article One (Absolute Version)
6+
7+
This is the ABSOLUTE version in fixtures/content directory.
8+
If this file is being resolved, the prioritization is CORRECT!
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
title: Article Two
3+
---
4+
5+
# Article Two
6+
7+
This is the second test article.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
title: Article Three
3+
---
4+
5+
# Article Three
6+
7+
This is the third test article in a subdirectory.

0 commit comments

Comments
 (0)