Skip to content

Commit e6c9b39

Browse files
authored
Fix orphaned features workflow to check variables files (#56183)
1 parent c987540 commit e6c9b39

File tree

9 files changed

+242
-1
lines changed

9 files changed

+242
-1
lines changed

src/data-directory/scripts/find-orphaned-features/find.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,12 @@ function searchAndRemove(features: Set<string>, pages: Page[], verbose = false)
155155
// them in, we'll need the English equivalent content to be able to
156156
// use the correctTranslatedContentStrings function.
157157

158+
// Check variables files
159+
for (const filePath of getVariableFiles(path.join(languages.en.dir, 'data', 'variables'))) {
160+
const fileContent = fs.readFileSync(filePath, 'utf-8')
161+
checkString(fileContent, features, { filePath, verbose, languageCode: 'en' })
162+
}
163+
158164
const englishReusables = new Map<string, string>()
159165
for (const filePath of getReusableFiles(path.join(languages.en.dir, 'data', 'reusables'))) {
160166
const relativePath = path.relative(languages.en.dir, filePath)
@@ -197,7 +203,7 @@ function searchAndRemove(features: Set<string>, pages: Page[], verbose = false)
197203
}
198204
}
199205

200-
function getReusableFiles(root: string): string[] {
206+
export function getReusableFiles(root: string): string[] {
201207
const here = []
202208
for (const file of fs.readdirSync(root)) {
203209
const filePath = `${root}/${file}`
@@ -210,6 +216,19 @@ function getReusableFiles(root: string): string[] {
210216
return here
211217
}
212218

219+
export function getVariableFiles(root: string): string[] {
220+
const here = []
221+
for (const file of fs.readdirSync(root)) {
222+
const filePath = `${root}/${file}`
223+
if (fs.statSync(filePath).isDirectory()) {
224+
here.push(...getVariableFiles(filePath))
225+
} else if (file.endsWith('.yml') && file !== 'README.yml') {
226+
here.push(filePath)
227+
}
228+
}
229+
return here
230+
}
231+
213232
const IGNORE_ARGS = new Set(['or', 'and', 'not', '<', '>', 'ghes', 'fpt', 'ghec', '!=', '='])
214233

215234
function checkString(
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import { fileURLToPath } from 'url'
2+
import path from 'path'
3+
import fs from 'fs'
4+
5+
import { describe, expect, test } from 'vitest'
6+
7+
const __dirname = path.dirname(fileURLToPath(import.meta.url))
8+
const fixturesDir = path.join(__dirname, 'orphaned-features', 'fixtures')
9+
10+
// Import the actual helper functions from the orphaned features script
11+
const { getVariableFiles, getReusableFiles } = await import(
12+
'#src/data-directory/scripts/find-orphaned-features/find.js'
13+
)
14+
15+
describe('orphaned features detection', () => {
16+
test('getVariableFiles finds all yml files in variables directory', () => {
17+
const variablesDir = path.join(fixturesDir, 'data', 'variables')
18+
const variableFiles = getVariableFiles(variablesDir)
19+
20+
// Should find our test.yml file
21+
expect(variableFiles).toHaveLength(1)
22+
expect(variableFiles[0]).toMatch(/test\.yml$/)
23+
24+
// Verify the file exists and contains expected content
25+
const testVariableContent = fs.readFileSync(variableFiles[0], 'utf-8')
26+
expect(testVariableContent).toContain('used-in-variables')
27+
expect(testVariableContent).toContain('ifversion')
28+
})
29+
30+
test('getReusableFiles finds all md files in reusables directory', () => {
31+
const reusablesDir = path.join(fixturesDir, 'data', 'reusables')
32+
const reusableFiles = getReusableFiles(reusablesDir)
33+
34+
// Should find our test.md file
35+
expect(reusableFiles).toHaveLength(1)
36+
expect(reusableFiles[0]).toMatch(/test\.md$/)
37+
38+
// Verify the file exists and contains expected content
39+
const testReusableContent = fs.readFileSync(reusableFiles[0], 'utf-8')
40+
expect(testReusableContent).toContain('used-in-reusables')
41+
expect(testReusableContent).toContain('ifversion')
42+
})
43+
44+
test('variables files contain feature references that should be detected', () => {
45+
const variablesDir = path.join(fixturesDir, 'data', 'variables')
46+
const testVariableFile = path.join(variablesDir, 'test.yml')
47+
48+
expect(fs.existsSync(testVariableFile)).toBe(true)
49+
50+
const content = fs.readFileSync(testVariableFile, 'utf-8')
51+
52+
// Verify the test file has the expected feature usage patterns
53+
expect(content).toContain('{% ifversion used-in-variables %}')
54+
expect(content).toContain('test_variable_with_feature')
55+
expect(content).toContain('complex_variable')
56+
})
57+
58+
test('helper functions handle nested directories', () => {
59+
// Create a temporary nested structure to test
60+
const tempDir = path.join(__dirname, 'temp-nested-test')
61+
const nestedVariablesDir = path.join(tempDir, 'variables', 'nested')
62+
const nestedReusablesDir = path.join(tempDir, 'reusables', 'nested')
63+
64+
// Create directories
65+
fs.mkdirSync(nestedVariablesDir, { recursive: true })
66+
fs.mkdirSync(nestedReusablesDir, { recursive: true })
67+
68+
// Create test files
69+
fs.writeFileSync(path.join(nestedVariablesDir, 'nested.yml'), 'test: value')
70+
fs.writeFileSync(path.join(nestedReusablesDir, 'nested.md'), '# Test content')
71+
fs.writeFileSync(path.join(tempDir, 'variables', 'root.yml'), 'root: value')
72+
fs.writeFileSync(path.join(tempDir, 'reusables', 'root.md'), '# Root content')
73+
74+
try {
75+
// Test getVariableFiles with nested structure
76+
const variableFiles = getVariableFiles(path.join(tempDir, 'variables'))
77+
expect(variableFiles).toHaveLength(2)
78+
expect(variableFiles.some((f) => f.includes('nested.yml'))).toBe(true)
79+
expect(variableFiles.some((f) => f.includes('root.yml'))).toBe(true)
80+
81+
// Test getReusableFiles with nested structure
82+
const reusableFiles = getReusableFiles(path.join(tempDir, 'reusables'))
83+
expect(reusableFiles).toHaveLength(2)
84+
expect(reusableFiles.some((f) => f.includes('nested.md'))).toBe(true)
85+
expect(reusableFiles.some((f) => f.includes('root.md'))).toBe(true)
86+
} finally {
87+
// Clean up
88+
fs.rmSync(tempDir, { recursive: true, force: true })
89+
}
90+
})
91+
92+
test('helper functions ignore non-target files', () => {
93+
// Create a temporary directory with mixed file types
94+
const tempDir = path.join(__dirname, 'temp-mixed-files')
95+
fs.mkdirSync(tempDir, { recursive: true })
96+
97+
// Create various file types
98+
fs.writeFileSync(path.join(tempDir, 'test.yml'), 'yml: content')
99+
fs.writeFileSync(path.join(tempDir, 'test.md'), '# MD content')
100+
fs.writeFileSync(path.join(tempDir, 'test.json'), '{"json": true}')
101+
fs.writeFileSync(path.join(tempDir, 'test.txt'), 'text content')
102+
fs.writeFileSync(path.join(tempDir, 'README.yml'), 'readme: content')
103+
fs.writeFileSync(path.join(tempDir, 'README.md'), '# README')
104+
105+
try {
106+
// getVariableFiles should only find .yml files (excluding README.yml)
107+
const variableFiles = getVariableFiles(tempDir)
108+
expect(variableFiles).toHaveLength(1)
109+
expect(variableFiles[0]).toMatch(/test\.yml$/)
110+
111+
// getReusableFiles should only find .md files (excluding README.md)
112+
const reusableFiles = getReusableFiles(tempDir)
113+
expect(reusableFiles).toHaveLength(1)
114+
expect(reusableFiles[0]).toMatch(/test\.md$/)
115+
} finally {
116+
// Clean up
117+
fs.rmSync(tempDir, { recursive: true, force: true })
118+
}
119+
})
120+
121+
test('verify fix addresses the original issue scenario', () => {
122+
// This test simulates the original issue where features were used only in variables
123+
// but not detected by the orphaned features script
124+
125+
const variablesDir = path.join(fixturesDir, 'data', 'variables')
126+
const featuresDir = path.join(fixturesDir, 'data', 'features')
127+
128+
// Verify our test setup has the scenario described in the issue
129+
expect(fs.existsSync(path.join(featuresDir, 'used-in-variables.yml'))).toBe(true)
130+
expect(fs.existsSync(path.join(featuresDir, 'truly-orphaned.yml'))).toBe(true)
131+
132+
// Check that the variable file references the feature
133+
const variableContent = fs.readFileSync(path.join(variablesDir, 'test.yml'), 'utf-8')
134+
expect(variableContent).toContain('used-in-variables')
135+
136+
// Verify that the getVariableFiles function would find this file
137+
const variableFiles = getVariableFiles(variablesDir)
138+
expect(variableFiles.length).toBeGreaterThan(0)
139+
140+
// This proves that the fix would catch features used in variables files
141+
// because the orphaned features script now scans these files
142+
const foundFeatureUsage = variableFiles.some((filePath) => {
143+
const content = fs.readFileSync(filePath, 'utf-8')
144+
return content.includes('used-in-variables')
145+
})
146+
147+
expect(foundFeatureUsage).toBe(true)
148+
})
149+
150+
test('functions correctly identify different file types in same directory', () => {
151+
// Create a directory with both .yml and .md files to ensure each function
152+
// only picks up its target file types
153+
const tempDir = path.join(__dirname, 'temp-mixed-target-files')
154+
fs.mkdirSync(tempDir, { recursive: true })
155+
156+
// Create files that both functions might encounter
157+
fs.writeFileSync(
158+
path.join(tempDir, 'variables.yml'),
159+
'var: {% ifversion test-feature %}enabled{% endif %}',
160+
)
161+
fs.writeFileSync(
162+
path.join(tempDir, 'reusable.md'),
163+
'{% ifversion test-feature %}Reusable content{% endif %}',
164+
)
165+
fs.writeFileSync(path.join(tempDir, 'other.txt'), 'other content')
166+
167+
try {
168+
// Each function should only find its target file type
169+
const variableFiles = getVariableFiles(tempDir)
170+
const reusableFiles = getReusableFiles(tempDir)
171+
172+
expect(variableFiles).toHaveLength(1)
173+
expect(variableFiles[0]).toMatch(/variables\.yml$/)
174+
175+
expect(reusableFiles).toHaveLength(1)
176+
expect(reusableFiles[0]).toMatch(/reusable\.md$/)
177+
178+
// Verify no cross-contamination
179+
expect(variableFiles.some((f) => f.endsWith('.md'))).toBe(false)
180+
expect(reusableFiles.some((f) => f.endsWith('.yml'))).toBe(false)
181+
} finally {
182+
// Clean up
183+
fs.rmSync(tempDir, { recursive: true, force: true })
184+
}
185+
})
186+
})
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
title: Test article for orphaned features testing
3+
versions:
4+
feature: used-in-content
5+
---
6+
7+
This is a test article that uses the `used-in-content` feature.
8+
9+
{% ifversion used-in-content %}
10+
This content is only shown when the used-in-content feature is enabled.
11+
{% endif %}
12+
13+
Some regular content that's always available.
14+
15+
## Test section
16+
17+
{% ifversion used-in-content %}
18+
Another conditional section using the feature.
19+
{% else %}
20+
Fallback content when feature is not available.
21+
{% endif %}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
title: Test feature that is truly orphaned and should be detected
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
title: Test feature used in content
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
title: Test feature used in reusables
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
title: Test feature used in variables
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
This is a test reusable that uses a feature.
2+
3+
{% ifversion used-in-reusables %}
4+
This content is only shown when the used-in-reusables feature is enabled.
5+
{% endif %}
6+
7+
Some regular content that's always shown.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Test variables file for orphaned features testing
2+
test_variable_with_feature: '{% ifversion used-in-variables %}This variable uses a feature{% else %}Default value{% endif %}'
3+
another_variable: 'Static value without features'
4+
complex_variable: '{% ifversion used-in-variables %}Feature enabled{% elsif fpt %}Free tier{% else %}Other{% endif %}'

0 commit comments

Comments
 (0)