Skip to content

Commit

Permalink
Make commit information opt-in (#107)
Browse files Browse the repository at this point in the history
* Make commit information optional
* Adapt tests

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Jym77 and github-actions[bot] authored Dec 4, 2024
1 parent d543a0b commit 6717873
Show file tree
Hide file tree
Showing 13 changed files with 190 additions and 135 deletions.
49 changes: 49 additions & 0 deletions .changeset/long-forks-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
"@siteimprove/alfa-test-utils": minor
---

**Breaking:** `CommitInformation` is now optional and some of its properties have been rennamed; see the package's changelog for explanations and migration advice.

Previously, `SIP.upload` was automatically collecting some information about the latest `git` commit and sending it to the Siteimprove Intelligence Platform, unless opted out via the `includeGitInfo: false` option. This presented two main drawbacks:

1. (minor) This was heavily reliant on the directory being part of a `git` repository. For codebases that use a different version control system, not only this was useless, but no alternative was provided.
2. (major) This was heavily reliant on the Accessibility Code Checker running from a NodeJS environment, where access to the underlying filesystem and `git` was doable. Not only this prevented the Accessibility Code Checker to run seamlessly from other environments such as browser extensions or Cypress; but the mere fact of trying to bundle `SIP.upload` for such environments (e.g. with Webpack) was causing the build to fail.

As a consequence, this release reverts a bit the approach on that information. The commit information is still valuable and can be used to name or group tests in an organised way (e.g., to follow the number of issues in a feature branch), but it now has to be provided by the caller. A `git` helper is still provided since it is by far the most used version control system.

Concretely, the following changes have been made:

1. The `CommitInformation.GitOrigin` field has been renamed `CommitInformation.Origin`.
2. `SIP.upload` now accepts a `commitInformation` option, of type `CommitInformation`; this in an object that must at least contain a `BranchName` field with a string value. (note the starting uppercase in `BranchName`).
3. If no `commitInformation` is provided, but the `testName` is a function, it will resolve to the default "Unnamed" test.
4. The `includeGitInfo` options of `SIP.upload` has been removed; if a `CommitInformation` has been provided, it will automatically be uploaded.

In order to migrate from previous versions:

1. If you were using `includeGitInfo: false` to opt-out of it, simply remove the option as it is now an opt-in information.
2. If you were using a `testName` function; you need to provide the commit information to the call. For this, use:

````typescript
import { getCommitInformation } from "@siteimprove/alfa-test-utils/git.js";

const gitInformation = await getCommitInformation();

SIP.upload({
... // other options
commitInformation: gitInformation,
testName: (commitInfo) => ... // same function as previously
})
```
3. If you were not previously relying on the commit information but wish to keep uploading it, use:
````typescript
import { getCommitInformation } from "@siteimprove/alfa-test-utils/git.js";

const gitInformation = await getCommitInformation();

SIP.upload({
... // other options
commitInformation: gitInformation,
})
```

Siteimprove recommends that you provide some basic commit information as it opens possibilities for filtering of the results and better reporting in the Siteimprove Intelligence Platform. As of November 2024, we are currently not relying on this information in any place, but it is likely that we will provide features using it in the future; therefore it might be easier to start providing it immediately instead of revisiting the codebase later.
1 change: 1 addition & 0 deletions config/knip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const config: KnipConfig = {
],
},
"packages/alfa-cli": { entry: ["src/alfa.ts!"], project },
"packages/alfa-test-utils": { entry: [...entry, "src/report/git.ts!"], project },
"packages/alfa-webdriver": {
entry,
project,
Expand Down
23 changes: 9 additions & 14 deletions docs/review/api/alfa-test-utils.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,17 @@ export namespace Audit {
export function run(page: Page, options?: Options): Promise<Audit>;
}

// @public (undocumented)
// @public
export interface CommitInformation {
Author: string | undefined;
Author?: string;
BranchName: string;
CommitHash: string | undefined;
CommitTimestamp: string | undefined;
Email: string | undefined;
GitOrigin: string | undefined;
Message: string | undefined;
CommitHash?: string;
CommitTimestamp?: string;
Email?: string;
Message?: string;
Origin?: string;
}

// Warning: (ae-internal-missing-underscore) The name "getCommitInformation" should be prefixed with an underscore because the declaration is marked as @internal
//
// @internal (undocumented)
export function getCommitInformation(): Promise<Result<CommitInformation, string>>;

// @public
export class Logging implements Equatable, json.Serializable<Logging.JSON> {
// (undocumented)
Expand Down Expand Up @@ -272,11 +267,11 @@ export namespace SIP {
// (undocumented)
export interface Options {
apiKey?: string;
includeGitInfo?: boolean;
commitInformation?: CommitInformation;
pageTitle?: string | ((page: Page) => string);
pageURL?: string | ((page: Page) => string);
siteID?: string;
testName?: string | ((git: CommitInformation) => string);
testName?: string | ((commit: CommitInformation) => string);
userName?: string;
}
// @internal
Expand Down
2 changes: 1 addition & 1 deletion packages/alfa-test-utils/docs/usage/reporting/basic.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const pageReportURL = await SIP.upload(alfaResult, {

This returns a URL to a Siteimprove Intelligence Platform page report showing the audit results, embedded within a [`Result` object](https://github.com/Siteimprove/alfa/blob/main/docs/api/alfa-result.md) to handle errors, use `console.log(pageReportURL.getOrElse(() => pageReportURL.getErrUnsafe()));` to show its value.

> **Note:** By default, when ran from a `git` repository, this will upload some [basic git information](https://github.com/Siteimprove/alfa-integrations/blob/main/docs/api/alfa-test-utils.commitinformation.md) if you prefer not to do so, add the `includeGitInfo: false` option to the second argument of `SIP.upload`.
> **Note:** Siteimprove recommends that you include some basic information about the latest commit together with the upload, at least the branch name. This opens possibilities of grouping and reporting based on it, e.g. to follow the number of issues in a given branch. See [common configuration](./configuration.md#including-commit-information) for more information.
## Pretty printing results in the console

Expand Down
39 changes: 33 additions & 6 deletions packages/alfa-test-utils/docs/usage/reporting/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,41 @@ const pageReportURL = Audit.run(alfaPage, {
});
```

## Skipping git information
## Including commit information

If you prefer not to upload basic git information about the repository (origin URL; branch name; latest commit hash, author, and message), use the `includeGitInfo: false` option:
Siteimprove recommends that you include some basic information about the latest commit together with the upload, at least the branch name. This opens possibilities of grouping and reporting based on it, e.g. to follow the number of issues in a given branch.

```typescript
SIP.upload(alfaResult, {
userName: process.env.SI_USER_NAME!,
apiKey: process.env.SI_API_KEY!,
includeGitInfo: false,
const pageReportURL = Audit.run(alfaPage, {
rules: { include: Rules.aaFilter },
}).then((alfaResult) => {
SIP.upload(alfaResult, {
userName: process.env.SI_USER_NAME!,
apiKey: process.env.SI_API_KEY!,
commitInformation: {
BranchName: "main",
CommitHash: "a1b2c3d4",
},
});
});
```

See [the `CommitInforation` API](https://github.com/Siteimprove/alfa-integrations/blob/main/docs/api/alfa-test-utils.commitinformation.md) for all the allowed properties, only `BranchName` is mandatory.

If running from a `git` repository (in a NodeJS environment), this commit information can be extracted automatically:

```typescript
import { getCommitInformation } from "@siteimprove/alfa-test-utils/git.js";

const gitInformation = await getCommitInformation();

const pageReportURL = Audit.run(alfaPage, {
rules: { include: Rules.aaFilter },
}).then((alfaResult) => {
SIP.upload(alfaResult, {
userName: process.env.SI_USER_NAME!,
apiKey: process.env.SI_API_KEY!,
commitInformation: gitInformation,
});
});
```
2 changes: 2 additions & 0 deletions packages/alfa-test-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
".": "./dist/index.js",
"./audit": "./dist/audit/index.js",
"./audit.js": "./dist/audit/index.js",
"./git": "./dist/report/git.js",
"./git.js": "./dist/report/git.js",
"./report": "./dist/report/index.js",
"./report.js": "./dist/report/index.js"
},
Expand Down
41 changes: 41 additions & 0 deletions packages/alfa-test-utils/src/report/commit-information.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* Global structure of a version control metadata.
*
* @remarks
* This is mostly modelled after git, but should work fine with other VCS like
* svn or cvs. Notably, only the branch name is mandatory.
*
* @public
*/
export interface CommitInformation {
/**
* The origin's URL. This may vary depending on whether the repository was cloned
* using `http` or `ssh` protocol.
*/
Origin?: string;
/**
* The name of the current branch.
*/
BranchName: string;
/**
* The hash of the latest commit.
*/
CommitHash?: string;
/**
* The name of the author of the latest commit.
*/
Author?: string;
/**
* The email of the author of the latest commit.
*/
Email?: string;
/**
* The timestamp of the latest commit.
*/
CommitTimestamp?: string;
/**
* The message of the latest commit.
*/
Message?: string;
}

39 changes: 3 additions & 36 deletions packages/alfa-test-utils/src/report/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,9 @@ import { Err, Ok } from "@siteimprove/alfa-result";
import type { Result } from "@siteimprove/alfa-result";
import { simpleGit } from "simple-git";

const git = simpleGit();
import type { CommitInformation } from "./commit-information.js";

/**
* @public
*/
export interface CommitInformation {
/**
* The origin's URL. This may vary depending on whether the repository was cloned
* using `http` or `ssh` protocol.
*/
GitOrigin: string | undefined;
/**
* The name of the current branch.
*/
BranchName: string;
/**
* The hash of the latest commit.
*/
CommitHash: string | undefined;
/**
* The name of the author of the latest commit.
*/
Author: string | undefined;
/**
* The email of the author of the latest commit.
*/
Email: string | undefined;
/**
* The timestamp of the latest commit.
*/
CommitTimestamp: string | undefined;
/**
* The message of the latest commit.
*/
Message: string | undefined;
}
const git = simpleGit();

/** @internal */
export async function getCommitInformation(): Promise<
Expand All @@ -51,7 +18,7 @@ export async function getCommitInformation(): Promise<
const latest = await git.log({ "--max-count": 1 });

const value: CommitInformation = {
GitOrigin: origin?.refs?.fetch,
Origin: origin?.refs?.fetch,
BranchName: branch.current,
CommitHash: latest?.latest?.hash,
Author: latest?.latest?.author_name,
Expand Down
15 changes: 14 additions & 1 deletion packages/alfa-test-utils/src/report/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
export * from "./git.js";
// git.js **MUST NOT** be re-exported automatically
// It depends on simple-git, that runs in Node and uses node-specific functions
// imported through the `node:` protocol.
// Due to this:
// 1. It cannot properly run in other environments, like the browser; thus
// making it useless for browser extensions, and hard to use in Cypress.
// 2. The mere fact of trying to bundle it for such environments (with Webpack or
// the likes) may crash the bundler.
// Therefore, we do not export it as part of the main package. It is still
// provided as a separate helper with its own `exports` path and has to be
// explicitly included in projects that need it.
// export * from "./git.js";

export * from "./commit-information.js"
export * from "./logging.js";
export * from "./sip.js";
24 changes: 9 additions & 15 deletions packages/alfa-test-utils/src/report/sip.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Array } from "@siteimprove/alfa-array";
import { Element, Query } from "@siteimprove/alfa-dom";
import { Map } from "@siteimprove/alfa-map";
import { Option } from "@siteimprove/alfa-option";
import { Err, Ok } from "@siteimprove/alfa-result";
import type { Result } from "@siteimprove/alfa-result";
import type { Thunk } from "@siteimprove/alfa-thunk";
Expand All @@ -11,7 +12,7 @@ import axios from "axios";
import type { Agent as HttpsAgent } from "https";

import { Audit, type Performance } from "../audit/index.js";
import { type CommitInformation, getCommitInformation } from "./git.js";
import type { CommitInformation } from "./commit-information.js";

/**
* Interacting with Siteimprove Intelligence Platform (SIP) API.
Expand Down Expand Up @@ -118,19 +119,14 @@ export namespace SIP {

/**
* A name for the test (e.g. "AA conformance", …), or a function building a
* test name from the git commit information (e.g. the git hash or branch name).
* test name from the commit information (e.g. the commit hash, or the branch name).
*/
testName?: string | ((git: CommitInformation) => string);
testName?: string | ((commit: CommitInformation) => string);

/**
* Whether to upload git commit information to the Siteimprove Intelligence Platform
* (default: yes).
*
* @remarks
* If the directory is not in a git repository, or git is not installed,
* this will silently fail and not send any information.
* Information about the latest commit of a Version Control System.
*/
includeGitInfo?: boolean;
commitInformation?: CommitInformation;
}

/**
Expand Down Expand Up @@ -243,7 +239,7 @@ export namespace SIP {
? title(page())
: title;

const gitInfo = await getCommitInformation();
const commitInfo = Option.from(options.commitInformation);

const name = options.testName ?? Defaults.Name;
const TestName =
Expand All @@ -252,7 +248,7 @@ export namespace SIP {
typeof name === "string"
? name
: name !== undefined
? gitInfo.map(name).getOrElse(() => gitInfo.getErrOr(Defaults.Name))
? commitInfo.map(name).getOr(Defaults.Name)
: Defaults.Name;

const result: Payload = {
Expand All @@ -272,9 +268,7 @@ export namespace SIP {
Durations: toCamelCase(audit.durations.common),
};

if ((options.includeGitInfo ?? true) && gitInfo.isOk()) {
result.CommitInformation = gitInfo.get();
}
commitInfo.forEach((info) => (result.CommitInformation = info));

if (options.siteID !== undefined) {
result.SiteId = options.siteID;
Expand Down
7 changes: 4 additions & 3 deletions packages/alfa-test-utils/src/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
"audit/outcomes.ts",
"audit/performance.ts",
"audit/rules.ts",
"report/logging.ts",
"report/commit-information.ts",
"report/get-rule-title.ts",
"report/git.ts",
"report/index.ts",
"report/sip.ts",
"report/get-rule-title.ts"
"report/logging.ts",
"report/sip.ts"
]
}
4 changes: 2 additions & 2 deletions packages/alfa-test-utils/test/report/git.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ test("getCommitInformation reads info from git", async (t) => {

const values = actual.getUnsafe();

t.notEqual(values.GitOrigin, undefined);
t(values.GitOrigin!.includes("Siteimprove/alfa-integrations"));
t.notEqual(values.Origin, undefined);
t(values.Origin!.includes("Siteimprove/alfa-integrations"));

for (const property of [
"BranchName",
Expand Down
Loading

0 comments on commit 6717873

Please sign in to comment.