Copilot commented on code in PR #418:
URL: https://github.com/apache/logging-parent/pull/418#discussion_r2150388464
##########
.github/actions/generate-dependabot-changelog/src/pull_request.test.ts:
##########
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: Apache-2.0
+import { getAssociatedPullRequest } from "./pull_request";
+import { graphql } from "@octokit/graphql";
+
+jest.mock("@octokit/graphql");
+
+describe("getAssociatedPullRequest", () => {
+ const mockGraphql = graphql.defaults as jest.Mock;
+
+ beforeEach(() => {
+ jest.clearAllMocks();
+ });
+
+ it("should return the associated pull request when the query is successful",
async () => {
+ const mockResponse = {
+ repository: {
+ object: {
+ associatedPullRequests: {
+ nodes: [
+ {
+ number: 123,
+ url: "https://github.com/owner/repo/pull/123";,
+ },
+ ],
+ },
+ },
+ },
+ };
+
+ mockGraphql.mockReturnValueOnce(jest.fn().mockResolvedValue(mockResponse));
+
+ const owner = "owner";
+ const repo = "repo";
+ const sha = "abc123";
+ const token = "test-token";
+
+ const result = await getAssociatedPullRequest(owner, repo, sha, token);
+
+ expect(result).toEqual({
+ number: 123,
+ url: new URL("https://github.com/owner/repo/pull/123";),
+ });
+ });
+
+ it("should throw an error when the query fails", async () => {
+ const mockError = new Error("GraphQL query failed");
+ mockGraphql.mockReturnValueOnce(jest.fn().mockRejectedValue(mockError));
+
+ const owner = "owner";
+ const repo = "repo";
+ const sha = "abc123";
+ const token = "test-token";
+
+ await expect(
+ getAssociatedPullRequest(owner, repo, sha, token),
+ ).rejects.toThrow("GraphQL query failed");
+ });
+
+ it("should return null if there are no PRs associated", async () => {
+ [
+ null,
+ // Wrong `owner` or `repo` values
+ {
+ repository: null,
+ },
+ // Wrong `sha` value
+ {
+ repository: {
+ object: null,
+ },
+ },
+ // Sha is not a commit
+ {
+ repository: {
+ object: {},
+ },
+ },
+ // No associated PRs
+ {
+ repository: {
+ object: {
+ associatedPullRequests: {
+ nodes: [],
+ },
+ },
+ },
+ },
+ ].forEach(async (mockResponse) => {
Review Comment:
Using `Array.forEach` with an async callback can lead to unawaited promises
and unreliable tests. Consider using a `for...of` loop or `Promise.all` to
ensure each test case runs and completes properly.
##########
.github/actions/generate-dependabot-changelog/src/maven.test.ts:
##########
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: Apache-2.0
+import { getProjectVersion } from "./maven";
+
+jest.mock("fs/promises");
+import { readFile } from "fs/promises";
+const mockReadFile = readFile as jest.MockedFunction<typeof readFile>;
+
+describe("getProjectVersion", () => {
+ beforeEach(() => {
+ jest.clearAllMocks();
+ });
+
+ it("should throw an error if pom.xml does not exist", async () => {
+ mockReadFile.mockRejectedValue(new Error("File not found"));
+
+ await expect(getProjectVersion("/mock/path")).rejects.toThrow(
+ "Error reading pom.xml: File not found",
+ );
+ });
+
+ it("should throw an error if revision property is not found in pom.xml",
async () => {
+ [
+ "project></project>",
+ "<project><properties></properties></project>",
+ "<project><properties><revision></revision></properties></project>",
+ ].forEach((content) => {
Review Comment:
This test uses `forEach` for multiple assertions without awaiting each
promise. Switching to a `for...of` loop or using `Promise.all` will make the
test behavior deterministic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
