Skip to content

Commit f80fa0c

Browse files
heiskrCopilot
andauthored
🐞 Check renamed content files for missing redirects (#61358)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4252f4a commit f80fa0c

2 files changed

Lines changed: 29 additions & 5 deletions

File tree

.github/workflows/test-changed-content.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,16 @@ jobs:
4949
uses: tj-actions/changed-files@22103cc46bda19c2b464ffe86db46df6922fd323 # v47.0.5
5050
with:
5151
files: 'content/**'
52+
# Needed to expose `all_old_new_renamed_files` (old,new pairs for renames).
53+
# Without this, files git classifies as renames (status R) are invisible to
54+
# the deleted-file redirect check below and old URLs can silently 404.
55+
include_all_old_new_renamed_files: true
5256

5357
- name: Run tests
5458
env:
5559
CHANGED_FILES: ${{ steps.changed_files.outputs.all_modified_files }}
5660
DELETED_FILES: ${{ steps.changed_files.outputs.deleted_files }}
61+
# Space-separated `oldPath,newPath` pairs. The test treats the old paths
62+
# like deleted files so missing redirects on renames are caught.
63+
RENAMED_FILES: ${{ steps.changed_files.outputs.all_old_new_renamed_files }}
5764
run: npm test -- src/content-render/tests/render-changed-and-deleted-files.ts

src/content-render/tests/render-changed-and-deleted-files.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@
44
*
55
* - CHANGED_FILES
66
* - DELETED_FILES
7+
* - RENAMED_FILES
78
*
8-
* They both need to a whitespace-separated list of paths to content files.
9-
* For example:
9+
* `CHANGED_FILES` and `DELETED_FILES` are whitespace-separated lists of
10+
* paths to content files. `RENAMED_FILES` is a whitespace-separated list
11+
* of `oldPath,newPath` pairs (as emitted by tj-actions/changed-files
12+
* `all_old_new_renamed_files` output). For example:
1013
*
1114
* export CHANGED_FILES="content/get-started/index.md content/get-started/start-your-journey/hello-world.md"
15+
* export RENAMED_FILES="content/old/path.md,content/new/path.md"
1216
*
1317
* If any of the paths in there, split by ' ', don't match real files, the
1418
* test will fail before it even starts. Meaning, it will throw an error
@@ -43,7 +47,7 @@ const pageList = await loadPages(undefined, ['en'])
4347
const SDK_DOCS_PATH = 'content/copilot/how-tos/copilot-sdk/'
4448

4549
function getChangedContentFiles() {
46-
const deleted = new Set(getDeletedContentFiles())
50+
const deleted = new Set([...getDeletedContentFiles(), ...getRenamedOldContentFiles()])
4751
return getContentFiles(process.env.CHANGED_FILES).filter(
4852
(f) => !deleted.has(f) && !f.startsWith(SDK_DOCS_PATH),
4953
)
@@ -57,6 +61,16 @@ function getDeletedContentFiles() {
5761
})
5862
}
5963

64+
// Parse `RENAMED_FILES` from tj-actions/changed-files `all_old_new_renamed_files`
65+
// output. Each whitespace-separated entry is an `oldPath,newPath` pair. We return
66+
// the OLD paths so they can be checked the same way deleted files are: the test
67+
// will fail if the old URL 404s (i.e. no redirect was set up for the rename).
68+
function getRenamedOldContentFiles() {
69+
const raw = (process.env.RENAMED_FILES || '').split(/\s+/g).filter(Boolean)
70+
const oldPaths = raw.map((pair) => pair.split(',')[0]).filter(Boolean)
71+
return getContentFiles(oldPaths.join(' '))
72+
}
73+
6074
function getContentFiles(spaceSeparatedList: string | undefined): string[] {
6175
return (spaceSeparatedList || '').split(/\s+/g).filter((filePath) => {
6276
// This filters out things like '', or `data/foo.md` or `content/something/README.md`
@@ -109,7 +123,10 @@ describe('changed-content', () => {
109123
})
110124

111125
describe('deleted-content', () => {
112-
const deletedContentFiles = getDeletedContentFiles()
126+
// Renamed files (status `R` from git) don't appear in `DELETED_FILES`, but
127+
// the old path is just as gone from the user's perspective and needs a
128+
// redirect. Treat the old path of each rename the same as a deleted file.
129+
const deletedContentFiles = [...getDeletedContentFiles(), ...getRenamedOldContentFiles()]
113130

114131
// `test.each` will throw if the array is empty, so we need to add a dummy
115132
// when there are no deleted files in the environment.
@@ -140,7 +157,7 @@ describe('deleted-content', () => {
140157
const res = await head(href)
141158
const error =
142159
res.statusCode === 404
143-
? `The deleted file ${file as string} did not set up a redirect when deleted.`
160+
? `The deleted or renamed file ${file as string} did not set up a redirect.`
144161
: ''
145162
// Certain articles that are deleted and moved under a directory with the same article name
146163
// should just route to the subcategory page instead of redirecting (docs content team confirmed).

0 commit comments

Comments
 (0)