Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support declaring PR dependencies in PR description #490

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .lintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# files for tools like prettier and eslint to ignore
dist
coverage
coverage
*.repos
README.md
22 changes: 22 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,28 @@ For example, if your secret is called `REPO_TOKEN`:
import-token: ${{ secrets.REPO_TOKEN }}
```

### Interdependent pull requests or merge requests

This action allows declaring PR dependencies.
For example, this may be useful when your PR depends on PRs/MRs/branches from other repos for it to work or be properly tested.

Include links to PR dependencies in your PR's description using the following format:

```
action-ros-ci-dependency: https://github.com/user/some-repo/pull/123
action-ros-ci-dependency: https://gitlab.com/user/some-repo/-/merge_requests/123
action-ros-ci-dependency: https://github.com/user/some-repo/tree/some-branch
action-ros-ci-dependency: https://gitlab.com/user/some-repo/-/tree/some-branch
```

If a given repo is in one of the repos files (see the [`vcs-repo-file-url` option](#Overview)), its `version` will be changed to point to the PR/MR/branch.
If a given repo is not in any of the repos files, it will be added to a new file and cloned along with the other repos.
This is useful if you usually rely on binary packages instead of building package dependencies from source every time.

Both GitHub and GitLab links are supported.
GitHub PRs are checked out using a `pull/$ID/merge` ref, i.e. PR source branch merged into target branch.
GitLab MRs are checked out using a `merge-requests/$ID` ref, i.e. MR source branch *not* merged into target branch, which is equivalent to using the MR's source branch directly.

## License

The scripts and documentation in this project are released under the [Apache 2](LICENSE) license.
Expand Down
1 change: 1 addition & 0 deletions __tests__/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pr-dependencies-test/
17 changes: 17 additions & 0 deletions __tests__/pr-dependencies-expected/0.repos
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
repositories:
ros2/launch:
type: git
url: 'https://github.com/ros2/launch'
version: pull/460/merge
ros2/rcl_logging:
type: git
url: 'https://github.com/ros2/rcl_logging/'
version: pull/53/merge
ros2/rcl:
type: git
url: 'https://github.com/ros2/rcl.git'
version: master
ros2/rclcpp:
type: git
url: 'https://github.com/ros2/rclcpp.git'
version: pull/1500/merge
9 changes: 9 additions & 0 deletions __tests__/pr-dependencies-expected/1.repos
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
repositories:
ros-tracing/ros2_tracing:
type: git
url: 'https://gitlab.com/ros-tracing/ros2_tracing.git'
version: merge-requests/219
ros2/rcpputils:
type: git
url: 'https://github.com/ros2/rcpputils.git'
version: h-turtle
9 changes: 9 additions & 0 deletions __tests__/pr-dependencies-expected/extra.repos
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
repositories:
extra/0:
type: git
url: 'https://github.com/ros-tooling/action-ros-ci'
version: pull/490/merge
extra/1:
type: git
url: 'https://gitlab.com/ros-tracing/tracetools_analysis'
version: an/awesome-feature_branch2
17 changes: 17 additions & 0 deletions __tests__/pr-dependencies/0.repos
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
repositories:
ros2/launch:
type: git
url: https://github.com/ros2/launch
version: master
ros2/rcl_logging:
type: git
url: https://github.com/ros2/rcl_logging/
version: master
ros2/rcl:
type: git
url: https://github.com/ros2/rcl.git
version: master
ros2/rclcpp:
type: git
url: https://github.com/ros2/rclcpp.git
version: master
9 changes: 9 additions & 0 deletions __tests__/pr-dependencies/1.repos
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
repositories:
ros-tracing/ros2_tracing:
type: git
url: https://gitlab.com/ros-tracing/ros2_tracing.git
version: master
ros2/rcpputils:
type: git
url: https://github.com/ros2/rcpputils.git
version: master
218 changes: 218 additions & 0 deletions __tests__/ros-ci.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import * as core from "@actions/core";
import * as io from "@actions/io";
import * as fs from "fs";
import * as path from "path";
import * as yaml from "js-yaml";
import * as actionRosCi from "../src/action-ros-ci";
import * as dep from "../src/dependencies";
import { execBashCommand } from "../src/action-ros-ci";

jest.setTimeout(20000); // in milliseconds
Expand Down Expand Up @@ -39,3 +44,216 @@ describe("validate distribution test", () => {
expect(actionRosCi.validateDistros("apples", "bananas")).toBe(false);
});
});

describe("PR dependency management", () => {
const reposFilesDir = path.join(__dirname, "pr-dependencies");
const reposFilesTestDir = `${reposFilesDir}-test`;
const reposFilesExpectedDir = `${reposFilesDir}-expected`;

function toDep(
repoUrl: string,
type: "pr/mr" | "branch",
reference: string | number
): dep.PrDependency {
return { repoUrl: repoUrl, type: type, reference: reference };
}

it("should not do anything if not a PR", async () => {
let payload = {};
expect(dep.getPrDependencies(payload)).toEqual([]);
payload = { pull_request: {} };
expect(dep.getPrDependencies(payload)).toEqual([]);
payload = { pull_request: { body: "" } };
expect(dep.getPrDependencies(payload)).toEqual([]);
});

it("should extract dependencies from the PR body", async () => {
const bodyEmpty = `
Description of the changes.

Blah blah.
`;
let payload = {};
payload = { pull_request: { body: bodyEmpty } };
expect(dep.getPrDependencies(payload)).toEqual([]);

const body = `
Description of the changes.

Blah blah.

action-ros-ci-dependency: https://github.com/user/repo/pull/123
action-ros-ci-dependency : https://github.com/user/repo/pull/123
action-ros-ci-dependency:
action-ros-ci-dependency:
action-ros-ci-dependency: https://github.com/other-user/other-repo/pull/456/
action-ros-ci-dependency: https://github.com/other-user/other-repo/pull
action-ros-ci-dependency: http://github.com/some-user/some_repo/pull/78
action-ros-ci-dependency: http://github.com/some-user/some_repo/tree/some-user/some-branch2
action-ros-ci-dependency: https://gitlab.com/some-user/some_repo/-/merge_requests/9
action-ros-ci-dependency: https://gitlab.com/some-user/some_repo/-/tree/user/awesome_branch
action-ros-ci-dependency: github.com/some-user/some-repo/pull/9
action-ros-ci-dependency: http://gitlab.com/org/some-project/some_repo/-/merge_requests/1
`;
payload = { pull_request: { body: body } };
const expected: dep.PrDependency[] = [
toDep("https://github.com/user/repo", "pr/mr", 123),
toDep("https://github.com/other-user/other-repo", "pr/mr", 456),
toDep("http://github.com/some-user/some_repo", "pr/mr", 78),
toDep("https://gitlab.com/some-user/some_repo", "pr/mr", 9),
toDep("http://gitlab.com/org/some-project/some_repo", "pr/mr", 1),
toDep(
"http://github.com/some-user/some_repo",
"branch",
"some-user/some-branch2"
),
toDep(
"https://gitlab.com/some-user/some_repo",
"branch",
"user/awesome_branch"
),
];
// Order matters for equality here, otherwise it doesn't really matter
expect(dep.getPrDependencies(payload)).toEqual(expected);
});

async function prepareTestFiles(): Promise<string[]> {
if (fs.existsSync(reposFilesTestDir)) {
fs.rmdirSync(reposFilesTestDir, { recursive: true });
}
// Copy input files for them to be (possibly) modified
await io.cp(reposFilesDir, reposFilesTestDir, {
force: true,
recursive: true,
});
return fs.readdirSync(reposFilesTestDir);
}

function checkTestReposFiles(
fileNames: string[],
testDir: string,
expectedDir: string
): void {
for (const filename of fileNames) {
expect(filename.length).toBeGreaterThan(0);
const result = yaml.safeLoad(
fs.readFileSync(path.join(testDir, filename), "utf8")
);
const expected = yaml.safeLoad(
fs.readFileSync(path.join(expectedDir, filename), "utf8")
);
expect(result).toEqual(expected);
}
}

it("should work fine if there are no repos files and no dependencies", async () => {
expect(
dep.replaceDependencyVersions([], reposFilesTestDir, [])
).toBeUndefined();
});

it("should not modify repos files if there are no dependencies", async () => {
// Try to replace versions without dependencies
const filenames = await prepareTestFiles();
const filesPaths = filenames.map((f) => path.join(reposFilesTestDir, f));
expect(
dep.replaceDependencyVersions(filesPaths, reposFilesTestDir, [])
).toBeUndefined();

// Compare test files to original files
expect(filesPaths.length).toEqual(2);
expect(filenames.length).toEqual(2);
checkTestReposFiles(filenames, reposFilesTestDir, reposFilesDir);
});

it("should replace versions of dependencies in repos files and create an extra repos file", async () => {
const dependencies: dep.PrDependency[] = [
toDep("https://github.com/ros2/launch", "pr/mr", 460),
toDep("https://github.com/ros2/rcpputils", "branch", "h-turtle"),
toDep("https://github.com/ros2/rcl_logging", "pr/mr", 53),
toDep("http://github.com/ros2/rclcpp", "pr/mr", 1500),
toDep("https://github.com/ros-tooling/action-ros-ci", "pr/mr", 490),
toDep(
"https://gitlab.com/ros-tracing/tracetools_analysis",
"branch",
"an/awesome-feature_branch2"
),
toDep("https://gitlab.com/ros-tracing/ros2_tracing", "pr/mr", 219),
];

// Replace versions
const filenames = await prepareTestFiles();
const filesPaths = filenames.map((f) => path.join(reposFilesTestDir, f));
const replaceRet = dep.replaceDependencyVersions(
filesPaths,
reposFilesTestDir,
dependencies
);
expect(replaceRet).toEqual(path.join(reposFilesTestDir, "extra.repos"));

// Compare test files to expected files
filenames.push(path.basename(replaceRet || ""));
expect(filesPaths.length).toEqual(2);
expect(filenames.length).toEqual(3);
checkTestReposFiles(filenames, reposFilesTestDir, reposFilesExpectedDir);
});

it("should create an extra repos file for all dependencies if no repos files are given", async () => {
const dependencies: dep.PrDependency[] = [
toDep("https://github.com/ros-tooling/action-ros-ci", "pr/mr", 490),
toDep(
"https://gitlab.com/ros-tracing/tracetools_analysis",
"branch",
"an/awesome-feature_branch2"
),
];

// Replace versions
await prepareTestFiles();
const replaceRet = dep.replaceDependencyVersions(
[],
reposFilesTestDir,
dependencies
);
expect(replaceRet).toEqual(path.join(reposFilesTestDir, "extra.repos"));

// Compare test files to expected files
const filenames = [path.basename(replaceRet || "")];
checkTestReposFiles(filenames, reposFilesTestDir, reposFilesExpectedDir);
});

it("should create full URLs from dependencies", async () => {
expect(
dep.getFullUrlFromDependency(
toDep("https://github.com/some-user/some_repo", "pr/mr", 123)
)
).toEqual("https://github.com/some-user/some_repo/pull/123");
expect(
dep.getFullUrlFromDependency(
toDep("https://gitlab.com/some-user/some_repo", "pr/mr", 123)
)
).toEqual("https://gitlab.com/some-user/some_repo/-/merge_requests/123");
expect(
dep.getFullUrlFromDependency(
toDep(
"https://github.com/some-user/some_repo",
"branch",
"my/awesome-feature_branch42"
)
)
).toEqual(
"https://github.com/some-user/some_repo/tree/my/awesome-feature_branch42"
);
expect(
dep.getFullUrlFromDependency(
toDep(
"https://gitlab.com/some-user/some_repo",
"branch",
"my/awesome-feature_branch42"
)
)
).toEqual(
"https://gitlab.com/some-user/some_repo/-/tree/my/awesome-feature_branch42"
);
});
});
Loading