Add integration tests for Maven custom repository #6297
Add integration tests for Maven custom repository #6297Ranvin36 wants to merge 7 commits intoballerina-platform:masterfrom
Conversation
* Add tests covering pack/push/pull, building, dependency resolution and locking-mode scenarios. * The tests exercise pushing and pulling bala artifacts from Github packages, building projects offline and validating version resolution across soft/medium/hard/locked locking modes.
This aligns the deletion endpoint with where the test packages are published so cleanup affects the correct package namespace and yields clearer, expected behavior in CI runs.
| org="bctestorg" | ||
| name="pkg2" | ||
| version="1.0.0" | ||
| repository="github1" No newline at end of file |
There was a problem hiding this comment.
We must add a newline at the end of a file.
| [build-options] | ||
| observabilityIncluded = true | ||
|
|
||
| # [[dependency]] |
There was a problem hiding this comment.
We can remove these lines from the file
| version = "0.1.0" | ||
| readme = "Package.md" | ||
|
|
||
|
|
There was a problem hiding this comment.
Pull request overview
This pull request enhances integration test coverage for Maven custom repository functionality by adding comprehensive tests for packaging, publishing, and dependency resolution with GitHub Packages. The PR introduces new test packages (pkg1, pkg2, pkg3, myproject1), utility methods for version management and dependency manipulation, and extensive test cases that validate different locking modes (soft, medium, hard, locked) and version resolution behaviors.
Changes:
- Added test resource packages (pkg1, pkg2, pkg3, myproject1) with Ballerina configurations for Maven repository testing
- Introduced utility methods in MavenCustomRepoTestUtils for pack/push operations, version editing, and dependency management
- Added platform-specific executable support (bal.bat for Windows) in TestUtils
- Implemented comprehensive integration tests covering locking modes, version resolution, and backward compatibility scenarios
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
| project-api-tests/src/test/resources/maven-repos/pkg1/* | Test package 1 with version 0.1.0, basic Ballerina package structure |
| project-api-tests/src/test/resources/maven-repos/pkg2/* | Test package 2 with version 1.0.0, basic Ballerina package structure |
| project-api-tests/src/test/resources/maven-repos/pkg3/* | Test package 3 with version 1.0.0, depends on pkg2 |
| project-api-tests/src/test/resources/maven-repos/myproject1/* | Test project that imports pkg1 and pkg2 from GitHub Packages |
| project-api-tests/src/test/resources/maven-repos/pact/Ballerina.toml | Modified version from 0.2.0 to 0.1.0, added commented dependency declarations |
| project-api-tests/src/test/java/org/ballerina/projectapi/TestUtils.java | Added platform-specific bal executable support and executeCleanCommand method |
| project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTestUtils.java | Added utility methods for pack/push, version editing, dependency management, and main.bal generation |
| project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java | Expanded test suite with locking mode tests, version resolution tests, and backward compatibility tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| jsjj | |||
There was a problem hiding this comment.
The Package.md file contains meaningless content "jsjj". For test resources, it would be better to use descriptive placeholder text like "Package documentation for pkg2" or similar, to make the test resources more maintainable and professional.
| jsjj | |
| Package documentation for pkg2. |
| @Test(description = "Build Package With More Versions", dependsOnMethods = "testCase2_publishAdditionalVersionsForDeps") | ||
| public void testCase5_addVersionInBallerinaToml_buildSucceeds() throws IOException, InterruptedException { | ||
| // Update the package version in Ballerina.toml first, then build to pick up the change. | ||
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", "1.1.0"); | ||
| List<String> args = new ArrayList<>(); | ||
| Process build = executeBuildCommand(DISTRIBUTION_FILE_NAME, this.tempWorkspaceDirectory.resolve("myproject1"), | ||
| args, this.envVariables); | ||
| String buildErrors = getString(build.getErrorStream()); | ||
| if (!buildErrors.isEmpty()) { | ||
| Assert.fail(OUTPUT_CONTAIN_ERRORS + buildErrors); | ||
| } | ||
| String buildOutput = getString(build.getInputStream()); | ||
| String normalized = buildOutput.replace("\\", "/").replace("\r\n", "\n").replace("\r", "\n"); | ||
| String collapsed = normalized.replaceAll("\\s+", " "); | ||
| Assert.assertTrue(collapsed.contains("Generating executable") && collapsed.contains("target/bin/myproject1.jar"), | ||
| "Expected generating executable + target/bin/myproject1.jar in output. Actual output: " + buildOutput); | ||
|
|
||
| Path dependencyPath = this.tempWorkspaceDirectory.resolve("myproject1").resolve("Dependencies.toml"); | ||
| Optional<String> pkg2Version = MavenCustomRepoTestUtils.getPackageVersionFromDependencies(dependencyPath, "pkg2"); | ||
| Assert.assertTrue(pkg2Version.isPresent(), "Expected pkg2 to be present in Dependencies.toml"); | ||
| Assert.assertEquals(pkg2Version.get(), "1.1.1", "Package version is not matching with the expected package version"); | ||
| } | ||
|
|
||
| @Test(description = "Build Package With More Versions", dependsOnMethods = "testCase2_publishAdditionalVersionsForDeps") |
There was a problem hiding this comment.
The test description "Build Package With More Versions" is duplicated for both testCase5_addVersionInBallerinaToml_buildSucceeds and testCase5_addIncompatibleVersion_buildFails. These tests have different purposes and should have distinct, descriptive names such as "Build package with compatible version update" and "Build package with incompatible version fails" respectively.
| List<String> args, Map<String, String> envProperties) throws IOException, InterruptedException { | ||
| args.add(0, command); | ||
| args.add(0, TEST_DISTRIBUTION_PATH.resolve(distributionName).resolve("bin").resolve("bal").toString()); | ||
| // args.add(0, TEST_DISTRIBUTION_PATH.resolve(distributionName).resolve("bin").resolve("bal").toString()); |
There was a problem hiding this comment.
There is commented-out code that represents the old implementation. Since the new platform-specific executable logic is now in place and working, this commented line should be removed to improve code maintainability and avoid confusion.
| // args.add(0, TEST_DISTRIBUTION_PATH.resolve(distributionName).resolve("bin").resolve("bal").toString()); |
| observabilityIncluded = true | ||
|
|
||
| # [[dependency]] | ||
| # org="bctestorg" | ||
| # name="pkg1" | ||
| # version="0.1.0" | ||
|
|
||
| # [[dependency]] | ||
| # org="bctestorg" | ||
| # name="pkg2" | ||
| # version="1.0.0" No newline at end of file |
There was a problem hiding this comment.
The pact package Ballerina.toml contains commented-out dependency declarations for pkg1 and pkg2. Since these test resources are being newly added and the dependencies are not being used, these commented lines should either be removed for clarity or uncommented if they are intended to be used in future tests.
| observabilityIncluded = true | |
| # [[dependency]] | |
| # org="bctestorg" | |
| # name="pkg1" | |
| # version="0.1.0" | |
| # [[dependency]] | |
| # org="bctestorg" | |
| # name="pkg2" | |
| # version="1.0.0" | |
| observabilityIncluded = true |
| pasteStaticMainBalWithPkg1AndPkg2(projectDir); | ||
| } | ||
|
|
||
| @Test(description = "Push more packages to Github to test locking mode results" , dependsOnMethods = "testCase1_buildMyProject_assertDependenciesToml") |
There was a problem hiding this comment.
Missing space after comma in the Test annotation. Should be "description = "Push more packages to Github to test locking mode results", dependsOnMethods" with proper spacing for consistency with Java coding conventions.
| @Test(description = "Push more packages to Github to test locking mode results" , dependsOnMethods = "testCase1_buildMyProject_assertDependenciesToml") | |
| @Test(description = "Push more packages to Github to test locking mode results", dependsOnMethods = "testCase1_buildMyProject_assertDependenciesToml") |
| Pattern depPattern = Pattern.compile("(?s)\\[\\[dependency\\]\\].*?org\\s*=\\s*\"" + Pattern.quote(org) + "\".*?name\\s*=\\s*\"" + Pattern.quote(name) + "\".*?(?:version\\s*=\\s*\"[^\"]*\".*?|)", Pattern.CASE_INSENSITIVE); | ||
| Matcher matcher = depPattern.matcher(toml); | ||
| if (matcher.find()) { | ||
| return; | ||
| } else { |
There was a problem hiding this comment.
The ensureLegacyDependency method regex pattern and matching logic have issues. The regex pattern attempts to match a dependency block but doesn't capture version information properly, and the method always returns early if a matching org+name is found (line 287), regardless of whether the version matches the desired version. This means if a dependency exists with a different version, it won't be updated to the requested version, which could cause test failures when specific versions are required.
| Pattern depPattern = Pattern.compile("(?s)\\[\\[dependency\\]\\].*?org\\s*=\\s*\"" + Pattern.quote(org) + "\".*?name\\s*=\\s*\"" + Pattern.quote(name) + "\".*?(?:version\\s*=\\s*\"[^\"]*\".*?|)", Pattern.CASE_INSENSITIVE); | |
| Matcher matcher = depPattern.matcher(toml); | |
| if (matcher.find()) { | |
| return; | |
| } else { | |
| // Match a complete [[dependency]] block for the given org and name, capturing the version value if present. | |
| // The block extends up to the next [[dependency]] header or end-of-file. | |
| String depRegex = | |
| "(?is)" + // DOTALL + case-insensitive | |
| "\\[\\[dependency\\]\\]" + // block header | |
| "[^\\[]*?" + // content up to next '[' | |
| "org\\s*=\\s*\"" + Pattern.quote(org) + "\"[^\\[]*?" + // matching org | |
| "name\\s*=\\s*\"" + Pattern.quote(name) + "\"[^\\[]*?" + // matching name | |
| "(version\\s*=\\s*\"([^\"]*)\")?" + // optional version field (group 2 = value) | |
| "[^\\[]*?" + // rest of block | |
| "(?=\\z|\\[\\[dependency\\]\\])"; // stop at next block or EOF | |
| Pattern depPattern = Pattern.compile(depRegex); | |
| Matcher matcher = depPattern.matcher(toml); | |
| if (matcher.find()) { | |
| String existingVersion = matcher.group(2); // may be null if version field is absent | |
| if (existingVersion != null && existingVersion.equals(version)) { | |
| // Dependency with desired version already present; nothing to change. | |
| } else { | |
| // Replace the existing dependency block with the desired one (including updated version). | |
| StringBuilder updatedToml = new StringBuilder(); | |
| updatedToml.append(toml, 0, matcher.start()); | |
| updatedToml.append(desiredDepBlock); | |
| updatedToml.append(toml.substring(matcher.end())); | |
| Files.writeString(ballerinaTomlPath, updatedToml.toString()); | |
| } | |
| } else { | |
| // No dependency block for this org+name; append a new one. |
| if (Files.exists(mainBal)) { | ||
| String existing = Files.readString(mainBal); | ||
| if (existing.contains(import1) && existing.contains(import2) && existing.contains(import3) | ||
| && existing.contains(call1) && existing.contains(call2) && existing.contains(call3)) { | ||
| return; // already contains desired snippet | ||
| } | ||
| } |
There was a problem hiding this comment.
The pasteStaticMainBalWithAllPkgs method checks if the file already contains the required imports and calls but only returns early if ALL conditions are met. However, the snippet that gets written always uses a fixed order (pkg2, pkg1, pkg3) in both imports and calls. If the existing file has the imports in a different order or with different formatting, this check will fail and the file will be overwritten unnecessarily, potentially causing inconsistent behavior in tests.
| Assert.assertEquals(pkg2Version.get(),"1.1.1","Package version is not matching with the expected package version"); | ||
| } | ||
|
|
||
| @Test(description = "Build package with medium locking mode" , dependsOnMethods = "testCase2_publishAdditionalVersionsForDeps", groups = "testCase2" ) |
There was a problem hiding this comment.
Missing space after comma in the Test annotation parameters. Should be "description = "Build package with medium locking mode", dependsOnMethods" with proper spacing for consistency with Java coding conventions.
| import io.ballerina.toml.api.Toml; | ||
| import okhttp3.OkHttpClient; | ||
| import okhttp3.Request; | ||
| import okhttp3.Response; |
There was a problem hiding this comment.
The import 'okhttp3.Response' is unused and should be removed to keep the imports clean and reduce unnecessary dependencies.
| import okhttp3.Response; |
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", "2.0.0"); | ||
| List<String> args = new ArrayList<>(); | ||
| Process build = executeBuildCommand(DISTRIBUTION_FILE_NAME, this.tempWorkspaceDirectory.resolve("myproject1"), | ||
| args, this.envVariables); | ||
| String buildErrors = getString(build.getErrorStream()); | ||
| Assert.assertTrue(buildErrors.contains("incompatible with the version locked in Dependencies.toml"), | ||
| "Incompatible version error message not found in the build errors. Actual stderr: " + buildErrors); | ||
| if (!existingVersion.isEmpty()) { | ||
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", existingVersion); |
There was a problem hiding this comment.
The testCase5_addIncompatibleVersion_buildFails test attempts to restore the existing version after the test completes (line 664), but this restoration happens regardless of whether the build assertion passes or fails. If the assertion on line 661 fails, the test will still attempt to restore the version, which could mask test failures or leave the project in an unexpected state for subsequent tests. Consider using a try-finally block to ensure proper cleanup even when assertions fail.
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", "2.0.0"); | |
| List<String> args = new ArrayList<>(); | |
| Process build = executeBuildCommand(DISTRIBUTION_FILE_NAME, this.tempWorkspaceDirectory.resolve("myproject1"), | |
| args, this.envVariables); | |
| String buildErrors = getString(build.getErrorStream()); | |
| Assert.assertTrue(buildErrors.contains("incompatible with the version locked in Dependencies.toml"), | |
| "Incompatible version error message not found in the build errors. Actual stderr: " + buildErrors); | |
| if (!existingVersion.isEmpty()) { | |
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", existingVersion); | |
| try { | |
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", "2.0.0"); | |
| List<String> args = new ArrayList<>(); | |
| Process build = executeBuildCommand(DISTRIBUTION_FILE_NAME, | |
| this.tempWorkspaceDirectory.resolve("myproject1"), args, this.envVariables); | |
| String buildErrors = getString(build.getErrorStream()); | |
| Assert.assertTrue(buildErrors.contains("incompatible with the version locked in Dependencies.toml"), | |
| "Incompatible version error message not found in the build errors. Actual stderr: " + buildErrors); | |
| } finally { | |
| if (!existingVersion.isEmpty()) { | |
| updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", existingVersion); | |
| } |
- Replace silent `catch (IOException)` in setup with fail to surface errors - Add try/finally to restore package versions in incompatible-version test - Fix misleading assertion messages and test descriptions - Make internal helper methods private and improve annotation/spacing - Improve deletion handling and idempotent `main.bal` paste logic - Tighten TOML edits and legacy dependency handling in `MavenCustomRepoTestUtils.java` - Remove unused imports and re-enable packTrigger stderr assertion - Replace placeholder `Package.md` contents in test resources
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces extensive test expansions for Maven custom-repo scenarios: new test helpers (pack/push, TOML edits), BeforeGroups setup hooks, platform-aware CLI handling, many new test fixtures (pkg1/pkg2/pkg3/myproject1), expanded tests for locking modes and legacy compatibility, and enhanced cleanup/assertions. Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant BalCLI as "bal CLI"
participant MavenRepo as "Maven Repo (test fixture)"
participant FS as "Filesystem (project dirs)"
TestSuite->>FS: prepare workspace (pkg1/pkg2/pkg3/myproject1)
TestSuite->>BalCLI: packTrigger(pkgX) -- executes `bal pack`
BalCLI->>FS: read project files / Ballerina.toml
BalCLI-->>TestSuite: pack result (bala/jar)
TestSuite->>BalCLI: pushTrigger(pkgX, --repository)
BalCLI->>MavenRepo: upload package artifact
MavenRepo-->>BalCLI: push response
BalCLI-->>TestSuite: push result
TestSuite->>FS: validate Dependencies.toml / versions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java`:
- Around line 249-259: The helper updateVersionForPackage can return false when
a package name isn't found, so modify the call sites (e.g., in
beforeGroupTestCase2 and the other test setup methods that update package
versions) to assert the call succeeded: call updateVersionForPackage(...) and
assert its boolean result is true, failing with a clear message like
"updateVersionForPackage failed for pkg1/pkg2" so the test fails loudly if the
version change wasn't applied; do this for each place where
updateVersionForPackage is invoked.
- Around line 61-63: Add TestNG's single-threading at the class level by
annotating the MavenCustomRepoTest class with `@Test`(singleThreaded = true) so
all test methods in the class run serially; update imports to include
org.testng.annotations.Test if missing and place the annotation on the class
declaration for MavenCustomRepoTest to prevent concurrent execution that breaks
shared state (e.g., System.setProperty usage).
In `@project-api-tests/src/test/resources/maven-repos/pkg1/.devcontainer.json`:
- Around line 1-4: The JSON in .devcontainer.json is invalid due to a trailing
comma after the "extensions" array; remove the trailing comma so the object
contains only "image" and "extensions" keys with proper comma separation (ensure
the "extensions" line does not end with a comma) to produce valid JSON for
devcontainer parsing.
In `@project-api-tests/src/test/resources/maven-repos/pkg2/.devcontainer.json`:
- Around line 1-4: Remove the trailing comma after the "extensions" array in the
JSON object so the devcontainer config becomes valid JSON; locate the object
containing the "image" and "extensions" keys in .devcontainer.json and either
delete the comma after the extensions entry or add another valid property
following it.
In `@project-api-tests/src/test/resources/maven-repos/pkg3/.devcontainer.json`:
- Around line 1-4: The JSON in the object with keys "image" and "extensions"
contains a trailing comma after the "extensions" array which makes it invalid;
remove the trailing comma after the "extensions" entry so the object ends with
the closing brace only and validate the file (e.g., via a JSON linter) to ensure
the "image" and "extensions" properties form valid JSON.
🧹 Nitpick comments (1)
project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTestUtils.java (1)
358-390: Align pasteStaticMainBalWithPkg1AndPkg2 with its idempotent contract.
The Javadoc says idempotent, but the method always overwrites. Either add a content check (like pasteStaticMainBalWithAllPkgs) or update the doc to match behavior.♻️ Suggested idempotency check
String import1 = "import bctestorg/pkg1;"; String import2 = "import bctestorg/pkg2;"; String call1 = "pkg1:main();"; String call2 = "pkg2:main();"; + if (Files.exists(mainBal)) { + String existing = Files.readString(mainBal); + if (existing.contains(import1) && existing.contains(import2) + && existing.contains(call1) && existing.contains(call2)) { + return; // already contains desired snippet + } + } + String snippet = import2 + "\n" + import1 + "\n" + "\n" +
| // Run all methods in this class on a single thread to avoid parallel execution | ||
| public class MavenCustomRepoTest { | ||
|
|
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
find . -name "MavenCustomRepoTest.java" -type fRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java
Script executed:
head -100 ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java | cat -nRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
1 /*
2 * Copyright (c) 2023, WSO2 LLC. (http://wso2.com).
3 *
4 * Licensed under the Apache License, Version 2.0 (the "License");
5 * you may not use this file except in compliance with the License.
6 * You may obtain a copy of the License at
7 *
8 * http://www.apache.org/licenses/LICENSE-2.0
9 *
10 * Unless required by applicable law or agreed to in writing, software
11 * distributed under the License is distributed on an "AS IS" BASIS,
12 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13 * See the License for the specific language governing permissions and
14 * limitations under the License.
15 */
16
17 package org.ballerina.projectapi;
18
19 import org.testng.Assert;
20 import org.testng.annotations.AfterClass;
21 import org.testng.annotations.BeforeClass;
22 import org.testng.annotations.BeforeGroups;
23 import org.testng.annotations.Test;
24
25 import java.io.File;
26 import java.io.IOException;
27 import java.net.URI;
28 import java.net.URISyntaxException;
29 import java.nio.file.FileVisitResult;
30 import java.nio.file.Files;
31 import java.nio.file.Path;
32 import java.nio.file.Paths;
33 import java.nio.file.SimpleFileVisitor;
34 import java.nio.file.StandardCopyOption;
35 import java.nio.file.attribute.BasicFileAttributes;
36 import java.util.ArrayList;
37 import java.util.List;
38 import java.util.Map;
39 import java.util.Objects;
40 import java.util.Optional;
41
42 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.createSettingToml;
43 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.deleteArtifacts;
44 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.deleteFiles;
45 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.editVersionBallerinaToml;
46 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.getEnvVariables;
47 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.getString;
48 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.packTrigger;
49 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.pushTrigger;
50 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.pasteStaticMainBalWithAllPkgs;
51 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.pasteStaticMainBalWithPkg1AndPkg2;
52 import static org.ballerina.projectapi.MavenCustomRepoTestUtils.updateVersionForPackage;
53 import static org.ballerina.projectapi.TestUtils.DISTRIBUTION_FILE_NAME;
54 import static org.ballerina.projectapi.TestUtils.OUTPUT_CONTAIN_ERRORS;
55 import static org.ballerina.projectapi.TestUtils.executeBuildCommand;
56 import static org.ballerina.projectapi.TestUtils.executeCleanCommand;
57 import static org.ballerina.projectapi.TestUtils.executePackCommand;
58 import static org.ballerina.projectapi.TestUtils.executePullCommand;
59 import static org.ballerina.projectapi.TestUtils.executePushCommand;
60
61 // Run all methods in this class on a single thread to avoid parallel execution
62 public class MavenCustomRepoTest {
63
64 private static final String org = "bctestorg";
65 private static final String platform = "any";
66 private static final String PACKAGE_NAME = "pkg1";
67 static final String GITHUB_REPO_ID = "github1";
68 private static final String VERSION = "0.1.0";
69 private Path actualHomeDirectory;
70 private Path tempWorkspaceDirectory;
71 private Path actualHomeDirectoryClone;
72 private Map<String, String> envVariables;
73
74 `@BeforeClass`()
75 public void setUp() throws IOException, InterruptedException {
76 TestUtils.setupDistributions();
77 deleteArtifacts(org, "pkg1");
78 deleteArtifacts(org, "pkg2");
79 deleteArtifacts(org, "pkg3");
80 actualHomeDirectory = Paths.get(System.getProperty("user.home")).resolve(".ballerina");
81 actualHomeDirectoryClone = Files.createTempDirectory("bal-test-integration-packaging-home-")
82 .resolve(".ballerina");
83 Files.walkFileTree(actualHomeDirectory,
84 new MavenCustomRepoTest.Copy(actualHomeDirectory, actualHomeDirectoryClone));
85 deleteFiles(actualHomeDirectory, true);
86 tempWorkspaceDirectory = Files.createTempDirectory("bal-test-integration-packaging-workspace-");
87
88 createSettingToml(actualHomeDirectory);
89 System.setProperty("user.home", actualHomeDirectory.getParent().toString());
90 envVariables = TestUtils.addEnvVariables(getEnvVariables(), actualHomeDirectory);
91
92 // Copy test resources to temp workspace directory
93 try {
94 URI testResourcesURI = Objects.requireNonNull(getClass().getClassLoader()
95 .getResource("maven-repos")).toURI();
96 Files.walkFileTree(Paths.get(testResourcesURI), new MavenCustomRepoTest.Copy(Paths.get(testResourcesURI),
97 this.tempWorkspaceDirectory));
98 } catch (URISyntaxException e) {
99 Assert.fail("error loading resources");
100 }
Script executed:
sed -n '60,80p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java | cat -nRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
1
2 // Run all methods in this class on a single thread to avoid parallel execution
3 public class MavenCustomRepoTest {
4
5 private static final String org = "bctestorg";
6 private static final String platform = "any";
7 private static final String PACKAGE_NAME = "pkg1";
8 static final String GITHUB_REPO_ID = "github1";
9 private static final String VERSION = "0.1.0";
10 private Path actualHomeDirectory;
11 private Path tempWorkspaceDirectory;
12 private Path actualHomeDirectoryClone;
13 private Map<String, String> envVariables;
14
15 `@BeforeClass`()
16 public void setUp() throws IOException, InterruptedException {
17 TestUtils.setupDistributions();
18 deleteArtifacts(org, "pkg1");
19 deleteArtifacts(org, "pkg2");
20 deleteArtifacts(org, "pkg3");
21 actualHomeDirectory = Paths.get(System.getProperty("user.home")).resolve(".ballerina");
Script executed:
wc -l ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
827 ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java
Script executed:
grep -n "@Test\|@BeforeGroups\|updateVersionForPackage" ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java | head -30Repository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
52:import static org.ballerina.projectapi.MavenCustomRepoTestUtils.updateVersionForPackage;
105: `@Test`(description = "Push package to Github packages", enabled = false)
140: `@Test`(description = "Pull package from Github packages", dependsOnMethods = "testPushBalaGithub", enabled = false)
161: `@Test`(description = "Build a package offline using a module from Github packages",
173: `@Test`(description = "Build a package Online using a module from Github packages",
215: `@Test`(description = "Build a package with multiple dependencies from Github packages")
249: `@BeforeGroups`("testCase2")
253: updateVersionForPackage(projectDir, "pkg1", "0.1.0");
254: updateVersionForPackage(projectDir, "pkg2", "1.0.0");
261: `@Test`(description = "Push more packages to Github to test locking mode results",
305: `@Test`(description = "Build package with soft locking mode",
351: `@Test`(description = "Build package with medium locking mode",
399: `@Test`(description = "Build package with hard locking mode",
449: `@Test`(description = "Build package with locked mode", dependsOnMethods = "testCase2_publishAdditionalVersionsForDeps"
482: `@Test`(description = "Build package with backwards compatibility", dependsOnGroups = "testCase2")
539: updateVersionForPackage(ballerinaTOML, "pkg1", "1.0.0");
540: updateVersionForPackage(ballerinaTOML, "pkg2", "1.0.0");
552: `@BeforeGroups`("testCase4")
557: `@Test`(description = "Test case 4.1: Add dep3 and build with SOFT locking mode",
588: `@Test`(description = "Test case 4.2: Add dep3 and build with MEDIUM (default) locking mode",
632: `@Test`(description = "Test case 4.3: Add dep3 and build with HARD locking mode",
675: `@Test`(description = "Test case 4.4: Add dep3 and build with LOCKED locking mode",
721: `@Test`(description = "Add a new version for an existing dependency and build",
725: updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"),
750: `@Test`(description = "Add an incompatible version for an existing dependency and build should fail",
766: updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", "2.0.0");
775: updateVersionForPackage(this.tempWorkspaceDirectory.resolve("myproject1"), "pkg2", existingVersion);
Script executed:
sed -n '253,254p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
updateVersionForPackage(projectDir, "pkg1", "0.1.0");
updateVersionForPackage(projectDir, "pkg2", "1.0.0");
Script executed:
grep -n "updateVersionForPackage" ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTestUtils.java | head -5Repository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
249: static boolean updateVersionForPackage(Path sourceDirectory, String packageName, String version)
Script executed:
sed -n '249,270p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTestUtils.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
static boolean updateVersionForPackage(Path sourceDirectory, String packageName, String version)
throws IOException {
Path ballerinaTomlPath = sourceDirectory.resolve("Ballerina.toml");
List<String> lines = Files.readAllLines(ballerinaTomlPath);
for (int i = 0; i < lines.size(); i++) {
String trimmed = lines.get(i).trim();
// look for name = "pkg"
if (trimmed.startsWith("name") && trimmed.contains("\"" + packageName + "\"")) {
// search forward for version line until next section header
for (int j = i + 1; j < lines.size(); j++) {
String t = lines.get(j).trim();
if (t.startsWith("[")) {
// reached next section, insert version right after the name line
lines.add(i + 1, "version = \"" + version + "\"");
Files.write(ballerinaTomlPath, lines);
return true;
}
if (t.startsWith("version")) {
// replace existing version line
lines.set(j, "version = \"" + version + "\"");
Files.write(ballerinaTomlPath, lines);
Script executed:
sed -n '249,280p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTestUtils.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
static boolean updateVersionForPackage(Path sourceDirectory, String packageName, String version)
throws IOException {
Path ballerinaTomlPath = sourceDirectory.resolve("Ballerina.toml");
List<String> lines = Files.readAllLines(ballerinaTomlPath);
for (int i = 0; i < lines.size(); i++) {
String trimmed = lines.get(i).trim();
// look for name = "pkg"
if (trimmed.startsWith("name") && trimmed.contains("\"" + packageName + "\"")) {
// search forward for version line until next section header
for (int j = i + 1; j < lines.size(); j++) {
String t = lines.get(j).trim();
if (t.startsWith("[")) {
// reached next section, insert version right after the name line
lines.add(i + 1, "version = \"" + version + "\"");
Files.write(ballerinaTomlPath, lines);
return true;
}
if (t.startsWith("version")) {
// replace existing version line
lines.set(j, "version = \"" + version + "\"");
Files.write(ballerinaTomlPath, lines);
return true;
}
}
// reached EOF without finding version or next section: append version after name
lines.add(i + 1, "version = \"" + version + "\"");
Files.write(ballerinaTomlPath, lines);
return true;
}
}
return false; // package name not found
Script executed:
sed -n '74,115p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
`@BeforeClass`()
public void setUp() throws IOException, InterruptedException {
TestUtils.setupDistributions();
deleteArtifacts(org, "pkg1");
deleteArtifacts(org, "pkg2");
deleteArtifacts(org, "pkg3");
actualHomeDirectory = Paths.get(System.getProperty("user.home")).resolve(".ballerina");
actualHomeDirectoryClone = Files.createTempDirectory("bal-test-integration-packaging-home-")
.resolve(".ballerina");
Files.walkFileTree(actualHomeDirectory,
new MavenCustomRepoTest.Copy(actualHomeDirectory, actualHomeDirectoryClone));
deleteFiles(actualHomeDirectory, true);
tempWorkspaceDirectory = Files.createTempDirectory("bal-test-integration-packaging-workspace-");
createSettingToml(actualHomeDirectory);
System.setProperty("user.home", actualHomeDirectory.getParent().toString());
envVariables = TestUtils.addEnvVariables(getEnvVariables(), actualHomeDirectory);
// Copy test resources to temp workspace directory
try {
URI testResourcesURI = Objects.requireNonNull(getClass().getClassLoader()
.getResource("maven-repos")).toURI();
Files.walkFileTree(Paths.get(testResourcesURI), new MavenCustomRepoTest.Copy(Paths.get(testResourcesURI),
this.tempWorkspaceDirectory));
} catch (URISyntaxException e) {
Assert.fail("error loading resources");
}
publishBalaPackagesBeforeTests();
}
`@Test`(description = "Push package to Github packages", enabled = false)
public void testPushBalaGithub() throws IOException, InterruptedException {
List<String> args = new ArrayList<>();
Process build = executePackCommand(DISTRIBUTION_FILE_NAME, this.tempWorkspaceDirectory.resolve(PACKAGE_NAME),
args, this.envVariables);
String buildErrors = getString(build.getErrorStream());
if (!buildErrors.isEmpty()) {
Assert.fail(OUTPUT_CONTAIN_ERRORS + buildErrors);
}
String buildOutput = getString(build.getInputStream());
String normalizedOutput = buildOutput.replace("\\", "/").replace("\r\n", "\n").replace("\r", "\n");
Script executed:
grep -B2 "public class MavenCustomRepoTest" ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
// Run all methods in this class on a single thread to avoid parallel execution
public class MavenCustomRepoTest {
TestNG @Test annotation singleThreaded parameter class level behavior
In TestNG, singleThreaded is a class-level-only attribute of @Test:
@Test(singleThreaded = true)guarantees all test methods in that class run on the same thread, even if the suite is running withparallel="methods"(i.e., it prevents method-level parallelism within that class). It’s ignored if placed on a method. [1][2]- Putting
@Teston a class also changes discovery: all public methods of that class become TestNG test methods even if they are not annotated (you can still annotate individual methods to add/override attributes like groups). [3]
So, the common pattern:
`@Test`(singleThreaded = true)
public class MyTest {
public void testA() {} // runs as a test (public + class-level `@Test`)
public void helper() {} // ALSO runs as a test if public
}If you want singleThreaded but don’t want “all public methods become tests”, you generally need to avoid class-level @Test (e.g., keep helper methods non-public), because singleThreaded is only supported at class level. [2][3]
Sources: TestNG annotations docs [1], TestNG @Test Javadoc [2], TestNG docs on class-level @Test behavior [3].
Script executed:
sed -n '60,150p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java | grep -E "^\s+(public|private|protected)\s+(void|boolean|String|Path|Map|List|File)"Repository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
private Path actualHomeDirectory;
private Path tempWorkspaceDirectory;
private Path actualHomeDirectoryClone;
private Map<String, String> envVariables;
public void setUp() throws IOException, InterruptedException {
public void testPushBalaGithub() throws IOException, InterruptedException {
public void testPullBalaGithub() throws IOException, InterruptedException {
Script executed:
grep -n "^\s*public\|^\s*private" ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java | head -40Repository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
62:public class MavenCustomRepoTest {
64: private static final String org = "bctestorg";
65: private static final String platform = "any";
66: private static final String PACKAGE_NAME = "pkg1";
68: private static final String VERSION = "0.1.0";
69: private Path actualHomeDirectory;
70: private Path tempWorkspaceDirectory;
71: private Path actualHomeDirectoryClone;
72: private Map<String, String> envVariables;
75: public void setUp() throws IOException, InterruptedException {
106: public void testPushBalaGithub() throws IOException, InterruptedException {
141: public void testPullBalaGithub() throws IOException, InterruptedException {
163: public void testBuildBalaGithubOffline() throws IOException, InterruptedException {
175: public void testBuildBalaGithubOnline() throws IOException, InterruptedException {
193: private void publishBalaPackagesBeforeTests() throws IOException, InterruptedException {
216: public void testCase1_buildMyProject_assertDependenciesToml() throws IOException, InterruptedException {
250: public void beforeGroupTestCase2() throws IOException {
263: public void testCase2_publishAdditionalVersionsForDeps() throws IOException, InterruptedException {
307: public void testCase2_1_softLockingMode_resolvesLatestCompatible() throws IOException, InterruptedException {
353: public void testCase2_2_mediumLockingMode_resolvesConservative() throws IOException, InterruptedException {
401: public void testCase2_3_hardLockingMode_enforcesExact() throws IOException, InterruptedException {
451: public void testCase2_4_lockedMode_usesLockedVersions() throws IOException, InterruptedException {
483: public void testCase3_backwardCompatibility_legacyBallerinaToml() throws IOException, InterruptedException {
522: private void pinPkg1AndPkg2To100() throws IOException, InterruptedException {
553: public void beforeGroupTestCase4() throws IOException, InterruptedException {
559: public void testCase4_1_addDep3_softLocking() throws IOException, InterruptedException {
590: public void testCase4_2_addDep3_mediumLocking() throws IOException, InterruptedException {
634: public void testCase4_3_addDep3_hardLocking() throws IOException, InterruptedException {
677: public void testCase4_4_addDep3_lockedLocking() throws IOException, InterruptedException {
723: public void testCase5_addVersionInBallerinaToml_buildSucceeds() throws IOException, InterruptedException {
752: public void testCase5_addIncompatibleVersion_buildFails() throws IOException, InterruptedException {
781: public void cleanup() throws IOException {
796: private final Path fromPath;
797: private final Path toPath;
798: private final StandardCopyOption copyOption;
811: public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
821: public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
Script executed:
sed -n '60,65p' ./project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.javaRepository: ballerina-platform/ballerina-distribution
Repository: ballerina-platform/ballerina-distribution
Exit code: 0
stdout:
// Run all methods in this class on a single thread to avoid parallel execution
public class MavenCustomRepoTest {
private static final String org = "bctestorg";
private static final String platform = "any";
Add @Test(singleThreaded = true) annotation to enforce serial execution.
The comment alone doesn't prevent parallel execution. These tests mutate shared state (System.setProperty at line 89, temp directories), risking flakiness if run concurrently. TestNG's singleThreaded attribute is the proper mechanism to guarantee serial execution.
🔧 Proposed fix
// Run all methods in this class on a single thread to avoid parallel execution
+@Test(singleThreaded = true)
public class MavenCustomRepoTest {🤖 Prompt for AI Agents
In
`@project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java`
around lines 61 - 63, Add TestNG's single-threading at the class level by
annotating the MavenCustomRepoTest class with `@Test`(singleThreaded = true) so
all test methods in the class run serially; update imports to include
org.testng.annotations.Test if missing and place the annotation on the class
declaration for MavenCustomRepoTest to prevent concurrent execution that breaks
shared state (e.g., System.setProperty usage).
project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java
Show resolved
Hide resolved
| { | ||
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | ||
| "extensions": ["WSO2.ballerina"], | ||
| } |
There was a problem hiding this comment.
Fix invalid JSON trailing comma.
Line 3 ends with a trailing comma, which makes this JSON invalid and can break devcontainer parsing.
🛠️ Proposed fix
- "extensions": ["WSO2.ballerina"],
+ "extensions": ["WSO2.ballerina"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | |
| "extensions": ["WSO2.ballerina"], | |
| } | |
| { | |
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | |
| "extensions": ["WSO2.ballerina"] | |
| } |
🤖 Prompt for AI Agents
In `@project-api-tests/src/test/resources/maven-repos/pkg1/.devcontainer.json`
around lines 1 - 4, The JSON in .devcontainer.json is invalid due to a trailing
comma after the "extensions" array; remove the trailing comma so the object
contains only "image" and "extensions" keys with proper comma separation (ensure
the "extensions" line does not end with a comma) to produce valid JSON for
devcontainer parsing.
| { | ||
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | ||
| "extensions": ["WSO2.ballerina"], | ||
| } |
There was a problem hiding this comment.
Invalid JSON: trailing comma not allowed.
The trailing comma after the extensions array on line 3 is invalid JSON syntax. This will cause parsing errors if the devcontainer configuration is actually used.
🔧 Proposed fix
{
"image": "ballerina/ballerina-devcontainer:2201.6.0",
- "extensions": ["WSO2.ballerina"],
+ "extensions": ["WSO2.ballerina"]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | |
| "extensions": ["WSO2.ballerina"], | |
| } | |
| { | |
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | |
| "extensions": ["WSO2.ballerina"] | |
| } |
🤖 Prompt for AI Agents
In `@project-api-tests/src/test/resources/maven-repos/pkg2/.devcontainer.json`
around lines 1 - 4, Remove the trailing comma after the "extensions" array in
the JSON object so the devcontainer config becomes valid JSON; locate the object
containing the "image" and "extensions" keys in .devcontainer.json and either
delete the comma after the extensions entry or add another valid property
following it.
| { | ||
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | ||
| "extensions": ["WSO2.ballerina"], | ||
| } |
There was a problem hiding this comment.
Invalid JSON: trailing comma not allowed.
Same issue as in pkg2/.devcontainer.json — the trailing comma after the extensions array is invalid JSON syntax.
🔧 Proposed fix
{
"image": "ballerina/ballerina-devcontainer:2201.6.0",
- "extensions": ["WSO2.ballerina"],
+ "extensions": ["WSO2.ballerina"]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | |
| "extensions": ["WSO2.ballerina"], | |
| } | |
| { | |
| "image": "ballerina/ballerina-devcontainer:2201.6.0", | |
| "extensions": ["WSO2.ballerina"] | |
| } |
🤖 Prompt for AI Agents
In `@project-api-tests/src/test/resources/maven-repos/pkg3/.devcontainer.json`
around lines 1 - 4, The JSON in the object with keys "image" and "extensions"
contains a trailing comma after the "extensions" array which makes it invalid;
remove the trailing comma after the "extensions" entry so the object ends with
the closing brace only and validate the file (e.g., via a JSON linter) to ensure
the "image" and "extensions" properties form valid JSON.
- Wrap `updateVersionForPackage(...)` calls with `Assert.assertTrue(...)` in `project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java` so tests fail immediately when package entries are missing from `Ballerina.toml`. Applied in: beforeGroupTestCase2(), pinPkg1AndPkg2To100(), testCase5_addVersionInBallerinaToml_buildSucceeds(). - Replace silent setup IOException handling with `Assert.fail(...)` to surface setup errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java`:
- Around line 678-705: The test testCase4_4_addDep3_lockedLocking currently
pre-populates pkg3 by calling pasteStaticMainBalWithAllPkgs before the hard lock
build; change the sequence so the project is initialized with only pkg1+pkg2 by
using pasteStaticMainBalWithPkg1AndPkg2 (and removing Dependencies.toml as
already done), run the hard lock build via executeBuildCommand(..., args) where
args contains "--locking-mode=hard", verify it succeeds, and only after that
call pasteStaticMainBalWithAllPkgs to add pkg3 before running the locked build
with "--locking-mode=locked" so the locked-mode import addition assertion
remains valid.
🧹 Nitpick comments (1)
project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTestUtils.java (1)
365-389: MakepasteStaticMainBalWithPkg1AndPkg2idempotent (or fix the Javadoc).The Javadoc says it returns early if the file already contains the imports/calls, but the implementation always overwrites
main.bal. Either add the guard (likepasteStaticMainBalWithAllPkgs) or update the comment to match behavior.♻️ Proposed guard to match the documented behavior
String import1 = "import bctestorg/pkg1;"; String import2 = "import bctestorg/pkg2;"; String call1 = "pkg1:main();"; String call2 = "pkg2:main();"; + if (Files.exists(mainBal)) { + String existing = Files.readString(mainBal); + if (existing.contains(import1) && existing.contains(import2) + && existing.contains(call1) && existing.contains(call2)) { + return; // already contains desired snippet + } + }
| @Test(description = "Test case 4.4: Add dep3 and build with LOCKED locking mode", | ||
| dependsOnGroups = "testCase2", groups = "testCase4") | ||
| public void testCase4_4_addDep3_lockedLocking() throws IOException, InterruptedException { | ||
| Path projectDir = this.tempWorkspaceDirectory.resolve("myproject1"); | ||
| MavenCustomRepoTestUtils.pasteStaticMainBalWithPkg1AndPkg2(projectDir); | ||
| Files.deleteIfExists(projectDir.resolve("Dependencies.toml")); | ||
| pasteStaticMainBalWithAllPkgs(projectDir); | ||
|
|
||
| List<String> args = new ArrayList<>(); | ||
| args.add("--locking-mode=hard"); | ||
| Process buildCleanup = executeBuildCommand(DISTRIBUTION_FILE_NAME, projectDir, | ||
| args, this.envVariables); | ||
| String buildCleanupErrors = getString(buildCleanup.getErrorStream()); | ||
| if (!buildCleanupErrors.isEmpty()) { | ||
| Assert.fail(OUTPUT_CONTAIN_ERRORS + buildCleanupErrors); | ||
| } | ||
|
|
||
| pasteStaticMainBalWithAllPkgs(projectDir); | ||
|
|
||
| args = new ArrayList<>(); | ||
| args.add("--locking-mode=locked"); | ||
| Process build = executeBuildCommand(DISTRIBUTION_FILE_NAME, projectDir, args, this.envVariables); | ||
| String buildErrors = getString(build.getErrorStream()); | ||
| String buildOutput = getString(build.getInputStream()); | ||
| String normalized = buildOutput.replace("\\", "/").replace("\r\n", "\n") | ||
| .replace("\r", "\n"); | ||
| String collapsed = normalized.replaceAll("\\s+", " "); | ||
|
|
There was a problem hiding this comment.
Locked-mode test pre-populates pkg3, so the “new imports” assertion becomes invalid.
In Line 682–689 you call pasteStaticMainBalWithAllPkgs before the hard build, so Dependencies.toml already contains pkg3. The locked-mode build won’t be adding new imports, which conflicts with the expected error. Build the hard-locked state with pkg1+pkg2, then add pkg3 before the locked run.
🧩 Proposed fix to restore the intended sequence
MavenCustomRepoTestUtils.pasteStaticMainBalWithPkg1AndPkg2(projectDir);
Files.deleteIfExists(projectDir.resolve("Dependencies.toml"));
- pasteStaticMainBalWithAllPkgs(projectDir);
List<String> args = new ArrayList<>();
args.add("--locking-mode=hard");
Process buildCleanup = executeBuildCommand(DISTRIBUTION_FILE_NAME, projectDir,
args, this.envVariables);
@@
- pasteStaticMainBalWithAllPkgs(projectDir);
+ // Now add pkg3 imports to trigger locked-mode failure
+ pasteStaticMainBalWithAllPkgs(projectDir);🤖 Prompt for AI Agents
In
`@project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java`
around lines 678 - 705, The test testCase4_4_addDep3_lockedLocking currently
pre-populates pkg3 by calling pasteStaticMainBalWithAllPkgs before the hard lock
build; change the sequence so the project is initialized with only pkg1+pkg2 by
using pasteStaticMainBalWithPkg1AndPkg2 (and removing Dependencies.toml as
already done), run the hard lock build via executeBuildCommand(..., args) where
args contains "--locking-mode=hard", verify it succeeds, and only after that
call pasteStaticMainBalWithAllPkgs to add pkg3 before running the locked build
with "--locking-mode=locked" so the locked-mode import addition assertion
remains valid.
Build the hard-locked state using only pkg1 + pkg2 before adding pkg3 imports. Move pasteStaticMainBalWithAllPkgs(projectDir) to after the hard-mode build so the locked-mode assertion for "cannot add new imports" is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java`:
- Around line 766-779: The calls to updateVersionForPackage are currently
unverified and may silently fail; update both usages of updateVersionForPackage
(the initial call that sets pkg2 to "2.0.0" and the revert call in the finally
block) to assert their boolean return is true (e.g.,
Assert.assertTrue(updateVersionForPackage(...), "failed to update version for
pkg2")), and for the revert call only assert when existingVersion is not empty;
keep the surrounding executeBuildCommand/getString assertions unchanged.
🧹 Nitpick comments (4)
project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java (4)
76-79: Fix inconsistent indentation in setUp method.Lines 76-79 have inconsistent indentation compared to the rest of the method. The
deleteArtifactscalls are indented with extra spaces, andactualHomeDirectoryassignment has even more indentation.🧹 Suggested fix
public void setUp() throws IOException, InterruptedException { TestUtils.setupDistributions(); - deleteArtifacts(org, "pkg1"); - deleteArtifacts(org, "pkg2"); - deleteArtifacts(org, "pkg3"); - actualHomeDirectory = Paths.get(System.getProperty("user.home")).resolve(".ballerina"); + deleteArtifacts(org, "pkg1"); + deleteArtifacts(org, "pkg2"); + deleteArtifacts(org, "pkg3"); + actualHomeDirectory = Paths.get(System.getProperty("user.home")).resolve(".ballerina");
203-210: Consider checking packTrigger result for errors.The
packTriggercall on line 204 ignores the returnedProcess. If packing fails, this error would be silently ignored and the subsequent push might fail with a confusing message. Consider checking stderr similar to how push is validated.🛡️ Suggested improvement
- // packTrigger returns a Process; we don't use the process object, so don't assign it - packTrigger(pkg, this.tempWorkspaceDirectory, this.envVariables); + Process pack = packTrigger(pkg, this.tempWorkspaceDirectory, this.envVariables); + String packErrors = getString(pack.getErrorStream()); + if (!packErrors.isEmpty()) { + Assert.fail("Pack failed for " + pkg + ": " + packErrors); + }
290-292: Unused variableignoredPackOut.The variable
ignoredPackOuton line 291 is assigned but never used. Consider removing it or checking for pack errors similar to lines 268-283.🧹 Suggested fix
- Process p = packTrigger("pkg2", this.tempWorkspaceDirectory, this.envVariables); - String ignoredPackOut = getString(p.getInputStream()); - p = pushTrigger("pkg2", this.tempWorkspaceDirectory, this.envVariables); + packTrigger("pkg2", this.tempWorkspaceDirectory, this.envVariables); + Process p = pushTrigger("pkg2", this.tempWorkspaceDirectory, this.envVariables);
306-481: Consider extracting common test setup/assertion logic.The locking mode tests (testCase2_1 through testCase2_4) share significant boilerplate: deleting
Dependencies.toml, running clean, building with a specific locking mode, normalizing output, and asserting dependency versions. A helper method could reduce duplication:private void buildWithLockingMode(String mode, Map<String, String> expectedVersions) throws IOException, InterruptedException { // Common setup, build, and assertions }However, some repetition in integration tests can improve readability, so this is optional.
project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java
Show resolved
Hide resolved
- Wrap `updateVersionForPackage(...)` calls with `Assert.assertTrue(...)` in `project-api tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java` (incompatible-version test) so the test fails fast when the package entry isn't found in `Ballerina.toml`. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java`:
- Around line 450-481: The test testCase2_4_lockedMode_usesLockedVersions is
flaky because it assumes Dependencies.toml was hard-locked by
testCase2_3_hardLockingMode_enforcesExact; update the TestNG annotation on the
method testCase2_4_lockedMode_usesLockedVersions to add
"testCase2_3_hardLockingMode_enforcesExact" to its dependsOnMethods so TestNG
runs the hard-locking test before this one, ensuring the expected pkg1/pkg2
versions are present in Dependencies.toml.
project-api-tests/src/test/java/org/ballerina/projectapi/MavenCustomRepoTest.java
Outdated
Show resolved
Hide resolved
Add `testCase2_3_hardLockingMode_enforcesExact` to `dependsOnMethods` of `testCase2_4_lockedMode_usesLockedVersions` and reformat the `@Test` annotation for readability. Ensures the locked-mode test runs after the hard-lock setup.
|
2 participants
Purpose
Enhance MavenCustomRepoTest (project-api-tests) to increase integration coverage for Maven custom repositories. This PR introduces a new integration test suite that exercises creating, packing, pushing and pulling bala artifacts from a custom Maven repository and validates build and dependency resolution behavior across different locking modes.
Goals
Harden MavenCustomRepoTest by adding end-to-end integration tests for packing, pushing and pulling bala artifacts to Github Packages.
Validate build and dependency-resolution behavior across locking modes (soft, medium, hard, locked), including offline/online builds.
Ensure resolved versions are recorded in Dependencies.toml and test version publishing/resolution across multiple dependency versions.
Automation tests
Integration tests
Summary by CodeRabbit
Tests
Chores