Skip to content

Commit

Permalink
Link checker: external links' validation (Qiskit#337)
Browse files Browse the repository at this point in the history
### Summary

This PR adds support for the validation of external links using the link
checker. Tests for the validation have been included, and the CI test
has been extended to validate external links.

By default, the link checker only validates internal links, but we can
activate the external links validation using:

`npm run check:links -- --external`

Closes Qiskit#305

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
  • Loading branch information
arnaucasau and Eric-Arellano authored Nov 14, 2023
1 parent 001450d commit c0470de
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
- name: Spellcheck
run: npm run check:spelling
- name: Link checker
run: npm run check:links
run: npm run check:links -- --external
- name: Formatting
run: npm run check:fmt
- name: Typecheck
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,12 @@ This staging environment can be useful to see how the docs are rendering before
CI will check for broken links. You can also check locally:

```bash
# Only check for broken links
# Only check for internal broken links
npm run check:links

# Only check for internal and external broken links
npm run check:links -- --external

# Or, run all the checks
npm run check
```
Expand Down
2 changes: 1 addition & 1 deletion docs/api/migration-guides/qiskit-runtime.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ can get set up on either platform by following the steps in [Install and set up.
<details>
<summary>Should I modify the Qiskit Terra algorithms?</summary>

As of v0.22, [Qiskit Terra algorithms](https://github.com/Qiskit/qiskit-terra/tree/main/qiskit/algorithms) use Qiskit Runtime primitives. Thus, there is no need for users to
As of v0.22, [Qiskit Terra algorithms](https://github.com/Qiskit/qiskit/tree/stable/0.46/qiskit/algorithms) use Qiskit Runtime primitives. Thus, there is no need for users to
modify amplitude estimators or any other Qiskit Terra algorithms.

</details>
Expand Down
2 changes: 1 addition & 1 deletion docs/verify/stabilizer-circuit-simulation.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"Stabilizer circuits are important to the study of quantum error correction. Their classical simulability also makes them useful for verifying the output of quantum computers. For example, suppose you want to execute a quantum circuit that uses 100 qubits on a quantum computer. How do you know that the quantum computer is behaving correctly? A quantum circuit on 100 qubits is beyond the reach of brute-force classical simulation. By modifying your circuit so that it becomes a stabilizer circuit, you can run circuits on the quantum computer that have a similar structure to your desired circuit, but which you can simulate on a classical computer. By checking the output of the quantum computer on the stabilizer circuits, you can gain confidence that it is behaving correctly on the non-stabilizer circuits as well. See [*Evidence for the utility of quantum computing before fault tolerance*](https://www.nature.com/articles/s41586-023-06096-3) for an example of this idea in practice.\n",
"\n",
"\n",
"[Exact and noisy simulation with Qiskit Aer primitives](https://docs.quantum-computing.ibm.com/verify/simulate-with-qiskit-aer) shows how to use [Qiskit Aer](https://qiskit.org/ecosystem/aer/) to perform exact and noisy simulations of generic quantum circuits. Consider the example circuit used in that article, an 8-qubit circuit built using [EfficientSU2](https://docs.quantum-computing.ibm.com/api/qiskit/qiskit.circuit.library.EfficientSU2):"
"[Exact and noisy simulation with Qiskit Aer primitives](simulate-with-qiskit-aer) shows how to use [Qiskit Aer](https://qiskit.org/ecosystem/aer/) to perform exact and noisy simulations of generic quantum circuits. Consider the example circuit used in that article, an 8-qubit circuit built using [EfficientSU2](https://docs.quantum-computing.ibm.com/api/qiskit/qiskit.circuit.library.EfficientSU2):"
]
},
{
Expand Down
42 changes: 36 additions & 6 deletions scripts/commands/checkLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { Root } from "remark-mdx";
import rehypeRemark from "rehype-remark";
import rehypeParse from "rehype-parse";
import remarkGfm from "remark-gfm";
import yargs from "yargs/yargs";
import { hideBin } from "yargs/helpers";

// The links in the files are not searched to see if they are valid.
// The files need a list of links to be ignored, and when an asterisk
Expand All @@ -35,6 +37,24 @@ const SYNTHETIC_FILES: string[] = [
"docs/api/runtime/tags/programs.mdx",
];

interface Arguments {
[x: string]: unknown;
external: boolean;
}

const readArgs = (): Arguments => {
return yargs(hideBin(process.argv))
.version(false)
.option("external", {
type: "boolean",
demandOption: false,
default: false,
description:
"Should external links be checked? This slows down the script, but is useful to check.",
})
.parseSync();
};

function markdownFromNotebook(source: string): string {
let markdown = "";
for (let cell of JSON.parse(source).cells) {
Expand Down Expand Up @@ -152,6 +172,8 @@ function loadFiles(existingPaths: string[]): File[] {
}

async function main() {
const args = readArgs();

// Determine what files we have and separate them into files with links
// to read and files we don't need to parse.
const pathsWithLinks = await globby("docs/**/*.{ipynb,md,mdx}");
Expand All @@ -168,12 +190,20 @@ async function main() {
const otherFiles = loadFiles(pathsWithoutLinks);
const existingFiles = docsFiles.concat(otherFiles);

// Validate the links and print the results
// Validate the links
const results = await Promise.all(
linkList
.filter((link) => args.external || !link.isExternal)
.map((link) => link.checkLink(existingFiles)),
);

// Print the results
let allGood = true;
linkList.forEach((link) => {
const errorMessages = link.checkLink(existingFiles);
errorMessages.forEach((errorMessage) => console.error(errorMessage));
allGood &&= errorMessages.length == 0;
results.forEach((linkErrors) => {
linkErrors.forEach((errorMessage) => {
console.error(errorMessage);
allGood = false;
});
});

if (!allGood) {
Expand All @@ -183,4 +213,4 @@ async function main() {
console.log("\nNo links appear broken ✅\n");
}

main();
main().then(() => process.exit());
44 changes: 30 additions & 14 deletions scripts/lib/LinkChecker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,39 +62,39 @@ describe("Test the constructor of Link", () => {
});

describe("Validate links", () => {
test("Validate existing internal links with absolute path", () => {
test("Validate existing internal links with absolute path", async () => {
let testLink = new Link("/testpath", ["/testorigin.mdx"]);
let testFile = new File("docs/testpath.mdx", []);
const results = testLink.checkLink([testFile]);
const results = await testLink.checkLink([testFile]);
expect(results).toEqual([]);
});

test("Validate non-existing internal links with absolute path", () => {
test("Validate non-existing internal links with absolute path", async () => {
let testLink = new Link("/test-alternative-path", ["/testorigin.mdx"]);
let testFile = new File("docs/testpath.mdx", []);
const results = testLink.checkLink([testFile]);
const results = await testLink.checkLink([testFile]);
expect(results).toEqual([
"❌ /testorigin.mdx: Could not find link '/test-alternative-path'",
]);
});

test("Validate existing internal links with relative path", () => {
test("Validate existing internal links with relative path", async () => {
let testLink = new Link("../testpath", ["docs/test/testorigin.mdx"]);
let testFile = new File("docs/testpath.mdx", []);
const results = testLink.checkLink([testFile]);
const results = await testLink.checkLink([testFile]);
expect(results).toEqual([]);
});

test("Validate non-existing internal links with relative path", () => {
test("Validate non-existing internal links with relative path", async () => {
let testLink = new Link("../testpath", ["docs/testorigin.mdx"]);
let testFile = new File("docs/testpath.mdx", []);
const results = testLink.checkLink([testFile]);
const results = await testLink.checkLink([testFile]);
expect(results).toEqual([
"❌ docs/testorigin.mdx: Could not find link '../testpath'",
]);
});

test("Validate existing internal links with absolute path and multiple origin files", () => {
test("Validate existing internal links with absolute path and multiple origin files", async () => {
let testLink = new Link("/testpath", [
"docs/test/testorigin.mdx",
"docs/test/test2/testorigin.mdx",
Expand All @@ -103,11 +103,11 @@ describe("Validate links", () => {
]);
let testFile1 = new File("docs/testpath.mdx", []);
let testFile2 = new File("docs/test/test2/testpath.mdx", []);
const results = testLink.checkLink([testFile1, testFile2]);
const results = await testLink.checkLink([testFile1, testFile2]);
expect(results).toEqual([]);
});

test("Validate non-existing internal links with absolute path and multiple origin files", () => {
test("Validate non-existing internal links with absolute path and multiple origin files", async () => {
let testLink = new Link("/testpath", [
"docs/test/testorigin.mdx",
"docs/test/test2/testorigin.mdx",
Expand All @@ -116,7 +116,7 @@ describe("Validate links", () => {
]);
let testFile1 = new File("docs/test/testpath.mdx", []);
let testFile2 = new File("docs/test2/test3/testpath.mdx", []);
const results = testLink.checkLink([testFile1, testFile2]);
const results = await testLink.checkLink([testFile1, testFile2]);
expect(results).toEqual([
"❌ docs/test/testorigin.mdx: Could not find link '/testpath'",
"❌ docs/test/test2/testorigin.mdx: Could not find link '/testpath'",
Expand All @@ -125,7 +125,7 @@ describe("Validate links", () => {
]);
});

test("Validate internal links with relative path and multiple origin files", () => {
test("Validate internal links with relative path and multiple origin files", async () => {
let testLink = new Link("../testpath", [
"docs/test/testorigin.mdx",
"docs/test/test2/testorigin.mdx",
Expand All @@ -134,12 +134,28 @@ describe("Validate links", () => {
]);
let testFile1 = new File("docs/testpath.mdx", []);
let testFile2 = new File("docs/test/test2/testpath.mdx", []);
const results = testLink.checkLink([testFile1, testFile2]);
const results = await testLink.checkLink([testFile1, testFile2]);
expect(results).toEqual([
"❌ docs/test/test2/testorigin.mdx: Could not find link '../testpath'",
"❌ docs/test/test3/testorigin.mdx: Could not find link '../testpath'",
]);
});

test("Validate existing external links", async () => {
let testLink = new Link("https://github.com/Qiskit", ["/testorigin.mdx"]);
const results = await testLink.checkLink([]);
expect(results).toEqual([]);
});

test("Validate existing external links", async () => {
let testLink = new Link("https://github.com/QiskitNotExistingRepo", [
"/testorigin.mdx",
]);
const results = await testLink.checkLink([]);
expect(results).toEqual([
"❌ /testorigin.mdx: Could not find link 'https://github.com/QiskitNotExistingRepo'",
]);
});
});

describe("Generate the possible paths of a given link", () => {
Expand Down
51 changes: 36 additions & 15 deletions scripts/lib/LinkChecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,24 @@ export class Link {
return possibleFilePaths;
}

checkExternalLink(): boolean {
// External link checking not supported yet
return true;
/**
* Returns a string with the error found, or null
* if there are no errors.
*/
async checkExternalLink(): Promise<string | null> {
try {
const response = await fetch(this.value, {
headers: { "User-Agent": "prn-broken-links-finder" },
});

if (response.status >= 300) {
return `Could not find link '${this.value}'`;
}
} catch (error) {
return `Failed to fetch '${this.value}': ${(error as Error).message}`;
}

return null;
}

/**
Expand All @@ -99,23 +114,29 @@ export class Link {
* the link, true if the link is in `existingFiles`
* or is a valid external link, otherwise false
*/
checkLink(existingFiles: File[]): string[] {
async checkLink(existingFiles: File[]): Promise<string[]> {
const errorMessages: string[] = [];

if (this.isExternal) {
// External link checking not supported yet
if (!this.isExternal) {
// Internal link
this.originFiles.forEach((originFile) => {
if (!this.checkInternalLink(existingFiles, originFile)) {
errorMessages.push(
`❌ ${originFile}: Could not find link '${this.value}'`,
);
}
});

return errorMessages;
}

// Internal link
this.originFiles.forEach((originFile) => {
if (!this.checkInternalLink(existingFiles, originFile)) {
errorMessages.push(
`❌ ${originFile}: Could not find link '${this.value}'`,
);
}
});

// External link
const errorMessage = await this.checkExternalLink();
if (errorMessage) {
this.originFiles.forEach((originFile: string) => {
errorMessages.push(`❌ ${originFile}: ${errorMessage}`);
});
}
return errorMessages;
}
}

0 comments on commit c0470de

Please sign in to comment.