Create AddGatewayEnvironmentTestCase.java#14048
Open
SadevSaranga wants to merge 1 commit intowso2:masterfrom
Open
Create AddGatewayEnvironmentTestCase.java#14048SadevSaranga wants to merge 1 commit intowso2:masterfrom
SadevSaranga wants to merge 1 commit intowso2:masterfrom
Conversation
Add test for gateway environment with empty display name
SadevSaranga
requested review from
AnuGayan,
Arshardh,
chamilaadhi and
tharindu1st
as code owners
|
|
Contributor
WalkthroughA new integration test class is added to validate that creating a gateway environment without a display name succeeds, including initialization, test execution, and cleanup procedures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java (2)
42-42: Use a unique environment name to avoid rerun/parallel collisions.Line 42 uses a fixed name. Re-runs or parallel executions can fail if a prior artifact remains.
Suggested fix
- String name = "test-env-no-displayname"; + String name = "test-env-no-displayname-" + System.currentTimeMillis();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java` at line 42, The test uses a fixed environment name string "test-env-no-displayname" in AddGatewayEnvironmentTestCase (variable name) which can collide across runs; change the assignment of the name variable to produce a unique value each run (e.g., append System.currentTimeMillis() or a UUID to "test-env-no-displayname") so the environment name is unique for each test execution and avoid rerun/parallel collisions.
72-82: Remove PR metadata comment block from source.This block is PR bookkeeping, not test documentation, and adds noise to production test code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java` around lines 72 - 82, Remove the PR metadata comment block (the /* PR title: ... PR description: ... */ block) from the top of the AddGatewayEnvironmentTestCase source so only relevant test comments remain; locate the multi-line comment in the AddGatewayEnvironmentTestCase.java file and delete it, then run tests to ensure no formatting or compilation issues remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java`:
- Line 29: The public class name AddGatewayEnvironmentWithoutDisplayNameTestCase
does not match the filename AddGatewayEnvironmentTestCase.java; rename the class
to AddGatewayEnvironmentTestCase or rename the file to
AddGatewayEnvironmentWithoutDisplayNameTestCase.java so the public top-level
class and filename match, updating any references/tests that instantiate or
import the class (e.g., in test runners or other classes).
- Around line 56-58: In AddGatewayEnvironmentTestCase, assign environmentId from
addedEnvironment (environmentId = addedEnvironment.getId()) immediately after
obtaining addedEnvironment (or right after the create call) before performing
assertions so the `@AfterClass` cleanup can always see the ID even if an assertion
fails; keep the Assert.assertNotNull(addedEnvironment) and
Assert.assertEquals(addedEnvironment.getName(), name) checks after the
assignment.
- Line 55: The assignment to EnvironmentDTO is wrong because
restAPIAdmin.addEnvironment(environmentDTO) returns ApiResponse<EnvironmentDTO>;
change the variable type to ApiResponse<EnvironmentDTO> (or call .getData() on
the returned ApiResponse and assign that to EnvironmentDTO) so the types match;
update the code around the restAPIAdmin.addEnvironment call and any subsequent
usage (e.g., use response.getData() for EnvironmentDTO or rename the variable to
response) to compile cleanly.
---
Nitpick comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java`:
- Line 42: The test uses a fixed environment name string
"test-env-no-displayname" in AddGatewayEnvironmentTestCase (variable name) which
can collide across runs; change the assignment of the name variable to produce a
unique value each run (e.g., append System.currentTimeMillis() or a UUID to
"test-env-no-displayname") so the environment name is unique for each test
execution and avoid rerun/parallel collisions.
- Around line 72-82: Remove the PR metadata comment block (the /* PR title: ...
PR description: ... */ block) from the top of the AddGatewayEnvironmentTestCase
source so only relevant test comments remain; locate the multi-line comment in
the AddGatewayEnvironmentTestCase.java file and delete it, then run tests to
ensure no formatting or compilation issues remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2310dc82-c93b-47bb-b0d9-685807c38416
📒 Files selected for processing (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class AddGatewayEnvironmentWithoutDisplayNameTestCase extends APIMIntegrationBaseTest { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify filename ↔ public class name alignment for this test file.
fd 'AddGatewayEnvironmentTestCase.java' -t f -x sh -c '
echo "== $1 ==";
rg -nP "public\s+class\s+\w+" "$1"
' sh {}Repository: wso2/product-apim
Length of output: 319
Public class name does not match filename (compile blocker).
Line 29 declares public class AddGatewayEnvironmentWithoutDisplayNameTestCase, but the file is AddGatewayEnvironmentTestCase.java. In Java, a public top-level class must match the filename exactly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java`
at line 29, The public class name
AddGatewayEnvironmentWithoutDisplayNameTestCase does not match the filename
AddGatewayEnvironmentTestCase.java; rename the class to
AddGatewayEnvironmentTestCase or rename the file to
AddGatewayEnvironmentWithoutDisplayNameTestCase.java so the public top-level
class and filename match, updating any references/tests that instantiate or
import the class (e.g., in test runners or other classes).
| name, displayName, description, provider, false, vHostDTOList); | ||
|
|
||
| try { | ||
| EnvironmentDTO addedEnvironment = restAPIAdmin.addEnvironment(environmentDTO); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the signature and local call-site types.
fd 'RestAPIAdminImpl.java' -t f -x rg -nP '\bApiResponse<EnvironmentDTO>\s+addEnvironment\s*\(' {}
fd 'APIMIntegrationBaseTest.java' -t f -x rg -nP '\brestAPIAdmin\b' {}
fd 'AddGatewayEnvironmentTestCase.java' -t f -x rg -nP '\baddEnvironment\s*\(' {}Repository: wso2/product-apim
Length of output: 447
Fix type mismatch in addEnvironment assignment.
Line 55 assigns the return value of restAPIAdmin.addEnvironment(environmentDTO) to EnvironmentDTO, but the method returns ApiResponse<EnvironmentDTO>. This will not compile.
Suggested fix
- EnvironmentDTO addedEnvironment = restAPIAdmin.addEnvironment(environmentDTO);
+ ApiResponse<EnvironmentDTO> response = restAPIAdmin.addEnvironment(environmentDTO);
+ EnvironmentDTO addedEnvironment = response.getData();📝 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.
Suggested change
| EnvironmentDTO addedEnvironment = restAPIAdmin.addEnvironment(environmentDTO); | |
| ApiResponse<EnvironmentDTO> response = restAPIAdmin.addEnvironment(environmentDTO); | |
| EnvironmentDTO addedEnvironment = response.getData(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java`
at line 55, The assignment to EnvironmentDTO is wrong because
restAPIAdmin.addEnvironment(environmentDTO) returns ApiResponse<EnvironmentDTO>;
change the variable type to ApiResponse<EnvironmentDTO> (or call .getData() on
the returned ApiResponse and assign that to EnvironmentDTO) so the types match;
update the code around the restAPIAdmin.addEnvironment call and any subsequent
usage (e.g., use response.getData() for EnvironmentDTO or rename the variable to
response) to compile cleanly.
Comment on lines
+56
to
+58
| Assert.assertNotNull(addedEnvironment); | ||
| Assert.assertEquals(addedEnvironment.getName(), name); | ||
| environmentId = addedEnvironment.getId(); |
Contributor
There was a problem hiding this comment.
Capture environmentId before assertions to guarantee cleanup.
If an assertion fails before Line 58, @AfterClass cannot delete the created environment, leaving test residue.
Suggested fix
- Assert.assertNotNull(addedEnvironment);
- Assert.assertEquals(addedEnvironment.getName(), name);
- environmentId = addedEnvironment.getId();
+ Assert.assertNotNull(addedEnvironment);
+ environmentId = addedEnvironment.getId();
+ Assert.assertNotNull(environmentId);
+ Assert.assertEquals(addedEnvironment.getName(), name);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/AddGatewayEnvironmentTestCase.java`
around lines 56 - 58, In AddGatewayEnvironmentTestCase, assign environmentId
from addedEnvironment (environmentId = addedEnvironment.getId()) immediately
after obtaining addedEnvironment (or right after the create call) before
performing assertions so the `@AfterClass` cleanup can always see the ID even if
an assertion fails; keep the Assert.assertNotNull(addedEnvironment) and
Assert.assertEquals(addedEnvironment.getName(), name) checks after the
assignment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add test for gateway environment with empty display name
Summary by CodeRabbit