Fix off-by-one error in code fence transform (#12540)

The code fence transform was including contents after the final END directive twice. That was not covered by the tests, because none of the examples contained any content after the final END directive, and concatenating the empty string twice does not produce an observable difference in the test results.

This bug was due to an off-by-one error in the loop of the multiSplice function. The error has been fixed, and more test cases have been added.
feature/default_network_editable
Erik Marks 3 years ago committed by GitHub
parent 6f6576917c
commit 2e8752183f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      development/build/transforms/remove-fenced-code.js
  2. 115
      development/build/transforms/remove-fenced-code.test.js

@ -408,7 +408,9 @@ function multiSplice(toSplice, splicingIndices) {
// pushes the substring between each "end" index and the next "begin" index // pushes the substring between each "end" index and the next "begin" index
// to the array of retained substrings. // to the array of retained substrings.
if (splicingIndices.length > 2) { if (splicingIndices.length > 2) {
for (let i = 1; i < splicingIndices.length; i += 2) { // Note the boundary index of "splicingIndices.length - 1". This loop must
// not iterate over the last element of the array.
for (let i = 1; i < splicingIndices.length - 1; i += 2) {
retainedSubstrings.push( retainedSubstrings.push(
toSplice.substring(splicingIndices[i], splicingIndices[i + 1]), toSplice.substring(splicingIndices[i], splicingIndices[i + 1]),
); );

@ -201,6 +201,14 @@ describe('build/transforms/remove-fenced-code', () => {
), ),
).toStrictEqual(testData.validOutputs[buildType]); ).toStrictEqual(testData.validOutputs[buildType]);
expect(
removeFencedCode(
mockFileName,
buildType,
testData.validInputs.extraContentWithFences,
),
).toStrictEqual(testData.validOutputsWithExtraContent[buildType]);
// Ensure that the minimal input template is in fact valid // Ensure that the minimal input template is in fact valid
const minimalInput = getMinimalFencedCode(buildType); const minimalInput = getMinimalFencedCode(buildType);
expect( expect(
@ -216,6 +224,17 @@ describe('build/transforms/remove-fenced-code', () => {
testData.validInputs.withoutFences, testData.validInputs.withoutFences,
), ),
).toStrictEqual([testData.validInputs.withoutFences, false]); ).toStrictEqual([testData.validInputs.withoutFences, false]);
expect(
removeFencedCode(
mockFileName,
buildType,
testData.validInputs.extraContentWithoutFences,
),
).toStrictEqual([
testData.validInputs.extraContentWithoutFences,
false,
]);
}); });
}); });
@ -611,6 +630,43 @@ Conditionally_Included
///: END:ONLY_INCLUDE_IN ///: END:ONLY_INCLUDE_IN
`, `,
extraContentWithFences: `
///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
Conditionally_Included
///: END:ONLY_INCLUDE_IN
Always_Included
Always_Included
Always_Included
Always_Included
///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
Conditionally_Included
Conditionally_Included
Conditionally_Included
///: END:ONLY_INCLUDE_IN
Always_Included
Always_Included
Always_Included
///: BEGIN:ONLY_INCLUDE_IN(flask)
Conditionally_Included
Conditionally_Included
///: END:ONLY_INCLUDE_IN
Always_Included
Always_Included
Always_Included
///: BEGIN:ONLY_INCLUDE_IN(flask)
Conditionally_Included
Conditionally_Included
///: END:ONLY_INCLUDE_IN
Always_Included
Always_Included
Always_Included
`,
withoutFences: ` withoutFences: `
Always_Included Always_Included
Always_Included Always_Included
@ -624,6 +680,24 @@ Always_Included
Always_Included Always_Included
Always_Included Always_Included
`,
extraContentWithoutFences: `
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
`, `,
}, },
@ -651,6 +725,38 @@ Always_Included
Always_Included Always_Included
Always_Included Always_Included
`,
true,
],
},
validOutputsWithExtraContent: {
beta: [
`
///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
Conditionally_Included
///: END:ONLY_INCLUDE_IN
Always_Included
Always_Included
Always_Included
Always_Included
///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
Conditionally_Included
Conditionally_Included
Conditionally_Included
///: END:ONLY_INCLUDE_IN
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
Always_Included
`, `,
true, true,
], ],
@ -659,5 +765,14 @@ Always_Included
data.validOutputs.flask = [data.validInputs.withFences, false]; data.validOutputs.flask = [data.validInputs.withFences, false];
data.validOutputs.main = [data.validInputs.withoutFences, true]; data.validOutputs.main = [data.validInputs.withoutFences, true];
data.validOutputsWithExtraContent.flask = [
data.validInputs.extraContentWithFences,
false,
];
data.validOutputsWithExtraContent.main = [
data.validInputs.extraContentWithoutFences,
true,
];
return deepFreeze(data); return deepFreeze(data);
} }

Loading…
Cancel
Save