test(portal-ui): add portal ui e2e gate and expand coverage#5551
test(portal-ui): add portal ui e2e gate and expand coverage#5551nobodyiam merged 3 commits intoapolloconfig:masterfrom
Conversation
dosubot
bot
added
the
size:XXL
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| use: { | ||
| ...devices['Desktop Chrome'], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Expected an identifier and instead saw '}'.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| name: 'chromium', | ||
| use: { | ||
| ...devices['Desktop Chrome'], | ||
| }, |
There was a problem hiding this comment.
Expected an identifier and instead saw '}'.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| module.exports = defineConfig({ | ||
| testDir: './tests', | ||
| workers: 1, | ||
| retries, |
There was a problem hiding this comment.
'object short notation' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| @@ -0,0 +1,29 @@ | |||
| const { defineConfig, devices } = require('@playwright/test'); | |||
|
|
|||
| const retries = Number(process.env.PLAYWRIGHT_RETRIES || '0'); | |||
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| @@ -0,0 +1,29 @@ | |||
| const { defineConfig, devices } = require('@playwright/test'); | |||
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
'destructuring binding' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
|
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:
📝 WalkthroughWalkthroughAdds Playwright-based Portal UI end-to-end infrastructure and CI, a readiness script, extensive Playwright helpers and test suites, README and configs, a GitHub Actions workflow to build/start the assembly and run E2E tests, and many new Mockito/MockMvc unit tests for portal controllers. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(135,206,235,0.5)
participant Playwright as Playwright (E2E Client)
participant Portal as Apollo Portal (UI/API)
participant ConfigSvc as Config Service (API)
participant H2 as H2 Database
end
Playwright->>Portal: Sign in, create app/cluster/namespace/item
Portal->>H2: Persist app/namespace/item/release
H2-->>Portal: Ack
Playwright->>Portal: Publish release / create gray branch
Portal->>H2: Persist release/branch
H2-->>Portal: Release created
Playwright->>ConfigSvc: Fetch Apollo config (pull) for IP
ConfigSvc->>H2: Query released/gray config
H2-->>ConfigSvc: Config data
ConfigSvc-->>Playwright: Return config
sequenceDiagram
rect rgba(144,238,144,0.5)
participant GitHub as GitHub Actions
participant Maven as Maven (JDK17)
participant Apollo as Apollo Assembly (Java App)
participant Script as wait-for-ready.sh
participant Playwright as Playwright
participant Browser as Chromium
end
GitHub->>Maven: Build Apollo assembly (JDK17)
Maven-->>GitHub: Assembly JAR
GitHub->>Apollo: Start assembly (H2 in-memory)
Apollo-->>GitHub: Starting
GitHub->>Script: Run wait-for-ready.sh
Script->>Apollo: Poll health endpoints and warm-up
Apollo-->>Script: Ready
GitHub->>Playwright: Run E2E tests
Playwright->>Browser: Launch Chromium
Browser->>Apollo: Interact with UI
Playwright-->>GitHub: Test results
GitHub->>GitHub: Upload artifacts on failure
GitHub->>Apollo: Stop assembly (cleanup)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/portal-ui-e2e.yml:
- Around line 53-61: The JAR selection uses `ls | grep` which is fragile;
replace that pipeline by using shell globbing to expand
apollo-assembly/target/apollo-assembly-*.jar and then programmatically skip
names matching 'sources' or 'javadoc' (e.g. iterate the glob results and assign
the first non-matching filename to the JAR variable), ensuring you handle the
case of no matches; update the code where JAR is set (the current `JAR=$(ls -1
... | grep -vE 'sources|javadoc' | head -n 1)`) to use the glob + simple pattern
check instead of ls/grep.
In `@CHANGES.md`:
- Line 33: The changelog bullet "CI: Add portal UI Playwright e2e gate on PRs
with JDK 17" is missing a PR/issue link; update that entry to match other Apollo
2.5.0 lines by appending the corresponding PR reference in the same format (e.g.
"…gate on PRs with JDK 17 (`#1234`)" or the full PR URL), ensuring the text
exactly matches the existing entry and includes the PR number or URL for
traceability.
🧹 Nitpick comments (23)
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/OrganizationControllerTest.java (2)
27-33: Consider migrating to JUnit 5 (Jupiter).This test uses JUnit 4 (
org.junit.Test,@RunWith(MockitoJUnitRunner.class)). The JUnit 5 equivalent would use@ExtendWith(MockitoExtension.class)andorg.junit.jupiter.api.Test. I see the siblingPageSettingControllerTestfollows the same JUnit 4 pattern, so this is consistent within the PR — but worth aligning with JUnit 5 in a follow-up.♻️ JUnit 5 migration sketch
-import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class OrganizationControllerTest {As per coding guidelines, "use JUnit 5 as the default framework".
34-34: Missing class-level Javadoc.Per coding guidelines, new Java classes should include a short Javadoc describing the class purpose.
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ReleaseHistoryControllerTest.java (1)
19-20: Consider migrating to JUnit 5.This test uses JUnit 4 (
org.junit.Test,@RunWith(MockitoJUnitRunner.class),org.junit.Assert). The coding guidelines specify JUnit 5 as the default framework for new tests. While the project currently uses JUnit 4 across most controller tests, migrating this test to JUnit 5 aligns with the stated guidelines. The JUnit 5 equivalent would use@ExtendWith(MockitoExtension.class),org.junit.jupiter.api.Test, andorg.junit.jupiter.api.Assertions.♻️ JUnit 5 migration diff
-import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.ctrip.framework.apollo.portal.component.UnifiedPermissionValidator; import com.ctrip.framework.apollo.portal.entity.bo.ReleaseHistoryBO; import com.ctrip.framework.apollo.portal.environment.Env; import com.ctrip.framework.apollo.portal.service.ReleaseHistoryService; import java.util.Collections; import java.util.List; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class ReleaseHistoryControllerTest {Also applies to: lines 31-32, 37-37
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PrefixPathControllerTest.java (1)
19-31: Uses JUnit 4 instead of JUnit 5.The test uses
@RunWith(MockitoJUnitRunner.class),org.junit.Test, andorg.junit.Assert, which are JUnit 4 APIs. The equivalent JUnit 5 setup would use@ExtendWith(MockitoExtension.class),org.junit.jupiter.api.Test, andorg.junit.jupiter.api.Assertions.♻️ Proposed migration to JUnit 5
-import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; import javax.servlet.ServletContext; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.test.util.ReflectionTestUtils; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class PrefixPathControllerTest {As per coding guidelines,
**/src/test/java/**/*Test.java: "use JUnit 5 as the default framework".apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PageSettingControllerTest.java (2)
47-47: PreferassertTrueoverassertEquals(true, ...).Minor readability nit —
assertTrueis more idiomatic for boolean assertions and gives a clearer failure message.- assertEquals(true, result.isCanAppAdminCreatePrivateNamespace()); + assertTrue(result.isCanAppAdminCreatePrivateNamespace());
19-30: JUnit 4 used instead of JUnit 5.This test uses JUnit 4 APIs (
org.junit.Test,@RunWith(MockitoJUnitRunner.class)) while coding guidelines require JUnit 5 for new test files. However, nearly all other controller tests in this module (21 of 22) still use JUnit 4, with onlyConsumerControllerTest.javamigrated to JUnit 5. Consider aligning with JUnit 5 if the project is actively migrating to it; otherwise, staying consistent with existing controller tests is reasonable.To migrate when ready:
- Replace
org.junit.Testwithorg.junit.jupiter.api.Test- Replace
@RunWith(MockitoJUnitRunner.class)with@ExtendWith(MockitoExtension.class)- Update assertions to use
org.junit.jupiter.api.Assertionsapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SsoHeartbeatControllerTest.java (2)
31-31: Missing class-level Javadoc.As per coding guidelines,
**/*.java: "New Java classes should include a short Javadoc describing the class purpose."Add Javadoc
`@RunWith`(MockitoJUnitRunner.class) +/** Unit tests for {`@link` SsoHeartbeatController}. */ public class SsoHeartbeatControllerTest {
24-30: Add class Javadoc and consider JUnit 5 migration for consistency with coding guidelines.The test class is missing Javadoc. Per coding guidelines, new Java classes should include a short Javadoc describing the class purpose (e.g.,
/** Tests for SsoHeartbeatController. */).Additionally, this test uses JUnit 4 (
@RunWith(MockitoJUnitRunner.class),org.junit.Test) while coding guidelines specify JUnit 5 as the default for**/src/test/java/**/*Test.java. This aligns with the existing codebase where 95% of controller tests use JUnit 4, but consider migrating when the broader test suite is upgraded.JUnit 5 equivalent (for future reference)
-import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class SsoHeartbeatControllerTest {apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConfigsImportControllerTest.java (3)
78-79: Prefer imports over inline fully-qualified class names.Several types are used with full package paths inline instead of being imported:
java.io.InputStream,java.util.zip.ZipInputStream,java.util.List<Env>,com.ctrip.framework.apollo.core.enums.ConfigFileFormat,org.springframework.test.web.servlet.result.MockMvcResultMatchers. This hurts readability.♻️ Add these to the import block and use simple names
+import com.ctrip.framework.apollo.core.enums.ConfigFileFormat; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import java.io.InputStream; +import java.util.List; +import java.util.zip.ZipInputStream;Then replace inline usages, e.g.:
- verify(configsImportService).forceImportNamespaceFromFile(eq(Env.DEV), eq(expected), - any(java.io.InputStream.class)); + verify(configsImportService).forceImportNamespaceFromFile(eq(Env.DEV), eq(expected), + any(InputStream.class));- .andExpect( - org.springframework.test.web.servlet.result.MockMvcResultMatchers.status().isOk()); + .andExpect(status().isOk());Also applies to: 91-91, 106-106, 133-134
48-48: Missing class-level Javadoc.Per the coding guidelines, new Java classes should include a short Javadoc describing the class purpose.
As per coding guidelines, "New Java classes should include a short Javadoc describing the class purpose".
📝 Suggested Javadoc
+/** + * Unit tests for {`@link` ConfigsImportController}. + */ `@RunWith`(MockitoJUnitRunner.class) public class ConfigsImportControllerTest {
34-41: Use JUnit 5 instead of JUnit 4 per coding guidelines.This test file uses JUnit 4 (
@RunWith(MockitoJUnitRunner.class),@Before,@Test) while coding guidelines specify JUnit 5 as the default framework for new test classes. The project is currently transitioning from JUnit 4 (88% of apollo-portal tests) to JUnit 5 (12% of tests). For new test classes, prefer JUnit 5 with@ExtendWith(MockitoExtension.class),@BeforeEach, andassertThrowsinstead of@Test(expected=...).apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SignInControllerTest.java (2)
23-32: LGTM — test logic is clean and covers the key parameter combinations.Minor nit: the coding guidelines ask for a short Javadoc on new classes (e.g.,
/** Tests for {@linkSignInController}. */).As per coding guidelines,
**/*.java: "New Java classes should include a short Javadoc describing the class purpose".
19-21: Consider migrating to JUnit 5 to align with coding guidelines.The test currently uses JUnit 4 imports (
org.junit.Test,org.junit.Assert.assertEquals). The coding guidelines specify JUnit 5 as the default framework for new test files.♻️ Proposed migration to JUnit 5
-import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; -import org.junit.Test; +import org.junit.jupiter.api.Test;apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/InstanceControllerTest.java (1)
89-94: Minor: inconsistent assertion style — mix of static imports and FQN calls.Lines 92-94 use
org.junit.Assert.assertTrue(...)while the rest of the file uses statically importedassertEquals/assertSame. AddassertTrueto the static imports at the top for consistency.Proposed fix
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue;Then replace:
- org.junit.Assert.assertTrue(releaseIds.contains(1L)); - org.junit.Assert.assertTrue(releaseIds.contains(2L)); - org.junit.Assert.assertTrue(releaseIds.contains(3L)); + assertTrue(releaseIds.contains(1L)); + assertTrue(releaseIds.contains(2L)); + assertTrue(releaseIds.contains(3L));apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SystemInfoControllerTest.java (1)
61-65:ReflectionTestUtils.invokeMethodto trigger@PostConstructis pragmatic but fragile.This works but couples the test to the private method name
"init". If that method is ever renamed, the test will fail at runtime with an unhelpful error. This is an acceptable trade-off for unit tests without a Spring context — just noting for awareness.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/EnvControllerTest.java (1)
26-32: Guidelines specify JUnit 5, but apollo-portal tests are predominantly JUnit 4 — consider if consistency or migration is the priority.The coding guidelines specify JUnit 5 as the default for new tests. However, 58 of 67 test files (87%) in apollo-portal currently use JUnit 4 (
org.junit.Test,@RunWith). Using JUnit 4 in these new tests maintains consistency with the module's existing test suite.If the project is planning to migrate portal tests to JUnit 5, updating these new tests would align with that direction. Otherwise, maintaining consistency with existing tests is reasonable. This is not a blocking issue — primarily a stylistic choice between following the published guideline or matching current module conventions.
e2e/portal-e2e/tests/helpers/portal-helpers.js (3)
1229-1245:rollbackLatestReleasehardcodesLOCALenv, unlike other helpers.Line 1238 uses a hardcoded
/envs/LOCAL/releases/URL fragment for the rollback API wait, while every other helper in this file parameterizesenvviaoptions(defaulting toDEFAULT_ENV). If this helper is ever reused for a different environment, it will silently wait for the wrong response.Suggested fix
async function rollbackLatestRelease(page, options = {}) { - const { namespaceName = DEFAULT_NAMESPACE } = options; + const { namespaceName = DEFAULT_NAMESPACE, env = DEFAULT_ENV } = options; const namespacePanel = await locateNamespacePanel(page, namespaceName); await namespacePanel.locator('[ng-click="rollback(namespace)"]:visible').first().click(); await expect(page.locator('#rollbackModal')).toBeVisible({ timeout: 30000 }); await page.click('#rollbackModal button[type="submit"]'); await expect(page.locator('#rollbackAlertDialog')).toBeVisible({ timeout: 30000 }); - const rollbackResponse = waitForApiResponse(page, 'PUT', '/envs/LOCAL/releases/'); + const rollbackResponse = waitForApiResponse(page, 'PUT', `/envs/${env}/releases/`); await Promise.all([ rollbackResponse, page.click('#rollbackAlertDialog button.btn-danger'), ]); await expect(page.locator('.toast-success').first()).toBeVisible({ timeout: 30000 }); }
11-16:generateUniqueIdhas a small collision window.
Date.now()+ a 4-digit random suffix means two calls within the same millisecond have a ~1/10000 chance of collision. Withworkers: 1and serial test execution the practical risk is low, but if tests are ever parallelized or the helper is called in a tight loop, collisions could cause flaky failures.A simple mitigation would be to add a monotonic counter:
Suggested hardening
+let uniqueIdCounter = 0; + function generateUniqueId(prefix) { - const randomSuffix = Math.floor(Math.random() * 10000) - .toString() - .padStart(4, '0'); - return `${prefix}${Date.now()}${randomSuffix}`; + uniqueIdCounter += 1; + const randomSuffix = Math.floor(Math.random() * 10000) + .toString() + .padStart(4, '0'); + return `${prefix}${Date.now()}${uniqueIdCounter}${randomSuffix}`; }
3-4: Hardcoded credentials — acceptable for local/CI E2E but consider environment variables.
USERNAMEandPASSWORDare hardcoded. This is fine for a local H2-backed test environment, but if the suite is ever pointed at a shared staging instance, these would leak or fail. Consider reading them from environment variables with these as defaults.e2e/portal-e2e/tests/portal-priority.spec.js (1)
83-83:publicNamespaceSeeduses a weaker uniqueness scheme thangenerateUniqueId.
Date.now().toString().slice(-6)gives only ~1M distinct values and two rapid test runs could collide. Consider usinggenerateUniqueId('pub')for consistency with the rest of the suite.e2e/portal-e2e/tests/portal-core.spec.js (1)
84-89: Consider adding a descriptive message to the history length assertion for easier debugging.If this assertion fails, the default message won't reveal what
historiesactually contained.Suggested improvement
- expect(histories.length).toBeGreaterThanOrEqual(3); + expect(histories.length, + `Expected at least 3 release history entries, got ${histories.length}` + ).toBeGreaterThanOrEqual(3);e2e/portal-e2e/scripts/wait-for-ready.sh (1)
21-59: Warm-up creates orphan apps on every retry iteration.Each time the main loop retries (e.g., item or release POST fails),
warm_up_portal_admin_pathcreates a new app with a uniqueapp_idthat is never cleaned up. In a tight retry loop (every 3 seconds for up to 5 minutes), this could leave ~100 orphan warmup apps in the test database.This is unlikely to cause problems in an ephemeral CI environment with H2, but worth noting.
e2e/portal-e2e/tests/portal-configservice.spec.js (1)
23-125: Well-structured full-chain test covering publish → gray → rollback.The test thoroughly validates:
- Config propagation after publish (non-gray client)
- Notification ID monotonicity
- Gray vs. non-gray isolation after gray publish
- Rollback correctly restores the prior main release while preserving the gray release
One minor note: this is a long test (~100 lines). If any step fails mid-way, the error message may not immediately clarify which phase failed. Consider adding descriptive
test.step()blocks for each phase (publish → gray → rollback) to improve failure diagnostics in CI.
| { | ||
| name: 'chromium', | ||
| use: { | ||
| ...devices['Desktop Chrome'], |
There was a problem hiding this comment.
Expected '}' to match '{' from line 39 and instead saw 'devices'.
Expected '}' to match '{' from line 41 and instead saw '...'.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
|
|
||
| module.exports = defineConfig({ | ||
| testDir: './tests', | ||
| workers, |
There was a problem hiding this comment.
'object short notation' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| const { defineConfig, devices } = require('@playwright/test'); | ||
|
|
||
| const retries = Number(process.env.PLAYWRIGHT_RETRIES || '0'); | ||
| const workers = Number(process.env.PLAYWRIGHT_WORKERS || (process.env.CI ? '2' : '1')); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| { | ||
| name: 'chromium', | ||
| use: { | ||
| ...devices['Desktop Chrome'], |
There was a problem hiding this comment.
Expected '}' to match '{' from line 39 and instead saw 'devices'.
Expected '}' to match '{' from line 41 and instead saw '...'.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
|
|
||
| module.exports = defineConfig({ | ||
| testDir: './tests', | ||
| workers, |
There was a problem hiding this comment.
'object short notation' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| const { defineConfig, devices } = require('@playwright/test'); | ||
|
|
||
| const retries = Number(process.env.PLAYWRIGHT_RETRIES || '0'); | ||
| const workers = Number(process.env.PLAYWRIGHT_WORKERS || (process.env.CI ? '2' : '1')); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
- add portal-ui-e2e workflow and Playwright e2e suite\n- expand portal UI-facing controller test coverage\n- add config service full-chain e2e scenarios and docs\n- fix license headers and add PR-linked CHANGES entry\n- optimize e2e runtime via Playwright cache and CI parallel workers\n- clarify AGENTS rule for CHANGES.md PR-link requirement
nobodyiam
force-pushed
the
codex/portal-ui-e2e-pr
branch
from
9e11115 to
d4b8e88
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/portal-ui-e2e.yml:
- Around line 78-82: Update the CI step that calls the Playwright installer:
change the command invocation "npx playwright install chromium" in the run block
to include the --with-deps flag (i.e., "npx playwright install --with-deps
chromium") so the installer also installs required system dependencies for
Chromium.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SignInControllerTest.java`:
- Around line 19-21: The test SignInControllerTest is importing JUnit 4
(org.junit.Test and org.junit.Assert.assertEquals); update it to JUnit 5 by
replacing the imports with org.junit.jupiter.api.Test and
org.junit.jupiter.api.Assertions.assertEquals (or a static import of
org.junit.jupiter.api.Assertions.assertEquals) and remove any junit4-specific
imports/annotations so the class uses the JUnit 5 `@Test` and assertion APIs
consistently.
🧹 Nitpick comments (12)
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SignInControllerTest.java (1)
27-32: Consider splitting assertions into separate test methods for clarity.A single test method asserts three distinct scenarios (both null, error param, logout param). Splitting them would give clearer failure diagnostics and better documents intent. That said, this is a minor nit given the simplicity of the logic.
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ReleaseHistoryControllerTest.java (2)
38-38: Missing class-level Javadoc.Per guidelines, new Java classes should include a short Javadoc describing the class purpose.
+/** + * Unit tests for {`@link` ReleaseHistoryController}. + */ public class ReleaseHistoryControllerTest {As per coding guidelines,
**/*.java: "New Java classes should include a short Javadoc describing the class purpose".
31-37: Use JUnit 5 instead of JUnit 4.This test uses JUnit 4 APIs (
org.junit.Test,@RunWith(MockitoJUnitRunner.class),org.junit.Assert). The coding guideline specifies JUnit 5 as the default framework for new tests.♻️ Migrate to JUnit 5
-import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.ctrip.framework.apollo.portal.component.UnifiedPermissionValidator; import com.ctrip.framework.apollo.portal.entity.bo.ReleaseHistoryBO; import com.ctrip.framework.apollo.portal.environment.Env; import com.ctrip.framework.apollo.portal.service.ReleaseHistoryService; import java.util.Collections; import java.util.List; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class ReleaseHistoryControllerTest {apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PageSettingControllerTest.java (3)
47-47: PreferassertTrueoverassertEquals(true, ...).
assertEquals(true, result.isCanAppAdminCreatePrivateNamespace())is less idiomatic;assertTruereads better and gives a clearer failure message.♻️ Suggested change
- assertEquals(true, result.isCanAppAdminCreatePrivateNamespace()); + assertTrue(result.isCanAppAdminCreatePrivateNamespace());
30-31: Missing class-level Javadoc.Per project guidelines, new Java classes should include a short Javadoc.
📝 Suggested addition
`@RunWith`(MockitoJUnitRunner.class) +/** + * Unit tests for {`@link` PageSettingController}. + */ public class PageSettingControllerTest {As per coding guidelines,
**/*.java: "New Java classes should include a short Javadoc describing the class purpose".
19-31: Migrate to JUnit 5 as per project guidelines.The test uses
@RunWith(MockitoJUnitRunner.class)and JUnit 4 imports. The coding guidelines specify JUnit 5 as the default framework for new tests in**/src/test/java/**/*Test.java.♻️ Proposed migration to JUnit 5
-import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; import com.ctrip.framework.apollo.portal.component.config.PortalConfig; import com.ctrip.framework.apollo.portal.entity.vo.PageSetting; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class PageSettingControllerTest {apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/InstanceControllerTest.java (3)
49-50: JUnit 4 used; coding guidelines require JUnit 5 for new tests.This new test class uses
@RunWith(MockitoJUnitRunner.class),org.junit.Test, andorg.junit.Before(JUnit 4). The equivalent JUnit 5 setup uses@ExtendWith(MockitoExtension.class),@org.junit.jupiter.api.Test, and@BeforeEach.Also, the class is missing a Javadoc comment describing its purpose.
♻️ Suggested migration to JUnit 5
-import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertSame; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; ... -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) +/** Tests for {`@link` InstanceController}. */ public class InstanceControllerTest {For
shouldRejectEmptyReleaseIds, replace@Test(expected = ...)withassertThrows:- `@Test`(expected = BadRequestException.class) + `@Test` public void shouldRejectEmptyReleaseIds() { - instanceController.getByReleasesNotIn("DEV", "SampleApp", "default", "application", ""); + assertThrows(BadRequestException.class, + () -> instanceController.getByReleasesNotIn("DEV", "SampleApp", "default", "application", "")); }As per coding guidelines, "Use JUnit 5 as the default framework" and "New Java classes should include a short Javadoc describing the class purpose".
83-94:ArgumentCaptor.capture()used in stubbing — prefer using it inverify()instead.Mockito discourages using
ArgumentCaptorinsidewhen()stubbing. The recommended pattern is to useany()(oranySet()) in the stub and capture withverify():♻️ Suggested refactor
- when(instanceService.getByReleasesNotIn(eq(Env.DEV), eq("SampleApp"), eq("default"), - eq("application"), releaseIdsCaptor.capture())).thenReturn(expected); + when(instanceService.getByReleasesNotIn(eq(Env.DEV), eq("SampleApp"), eq("default"), + eq("application"), org.mockito.ArgumentMatchers.anySet())).thenReturn(expected); List<InstanceDTO> result = instanceController.getByReleasesNotIn("DEV", "SampleApp", "default", "application", "1,2,2,3"); assertSame(expected, result); - Set<Long> releaseIds = releaseIdsCaptor.getValue(); + verify(instanceService).getByReleasesNotIn(eq(Env.DEV), eq("SampleApp"), eq("default"), + eq("application"), releaseIdsCaptor.capture()); + Set<Long> releaseIds = releaseIdsCaptor.getValue();Also, lines 92–93 use fully-qualified
org.junit.Assert.assertTruewhile other assertions use static imports — keep a consistent style.
110-110: UsegetStatusCode().value()for forward compatibility with Spring Framework 6.While
getStatusCodeValue()works in the current Spring Boot 2.7.11, it is deprecated in Spring Framework 6 and scheduled for removal in Spring 7. AdoptgetStatusCode().value()to prepare for future upgrades.- assertEquals(200, response.getStatusCodeValue()); + assertEquals(200, response.getStatusCode().value());Note: This pattern appears in multiple test files across apollo-portal (InstanceControllerTest, ClusterControllerTest, PermissionControllerTest).
e2e/portal-e2e/scripts/wait-for-ready.sh (1)
37-75: Warm-up app data is never cleaned up.The function creates an app (
warmup...), a config item, and a release that persist in the H2 database. While this won't affect correctness in CI (disposable H2), it will leave stale data for local development runs. Consider adding a comment noting this, or add a cleanup step that deletes the warmup app after success.e2e/portal-e2e/tests/helpers/portal-helpers.js (1)
1245-1261:rollbackLatestReleasehardcodes'LOCAL'env in the API URL fragment.The URL fragment on line 1254 is hardcoded to
/envs/LOCAL/releases/, but the function acceptsoptionsthat could theoretically include anenvparameter. If this helper is reused for a non-LOCAL environment, thewaitForApiResponsewill never match the actual response URL.Suggested fix
async function rollbackLatestRelease(page, options = {}) { - const { namespaceName = DEFAULT_NAMESPACE } = options; + const { namespaceName = DEFAULT_NAMESPACE, env = DEFAULT_ENV } = options; const namespacePanel = await locateNamespacePanel(page, namespaceName); ... - const rollbackResponse = waitForApiResponse(page, 'PUT', '/envs/LOCAL/releases/'); + const rollbackResponse = waitForApiResponse(page, 'PUT', `/envs/${env}/releases/`);e2e/portal-e2e/tests/portal-priority.spec.js (1)
99-99: Minor inconsistency:publicNamespaceSeedbypassesgenerateUniqueId.All other test identifiers use
generateUniqueId(prefix), but this line hand-rolls a timestamp suffix. If there's no specific naming constraint for public namespaces that forces this, prefer usinggenerateUniqueId('pub_')for consistency and reduced collision risk.- const publicNamespaceSeed = `pub${Date.now().toString().slice(-6)}`; + const publicNamespaceSeed = generateUniqueId('pub_');
| run: | | ||
| cd e2e/portal-e2e | ||
| npm ci | ||
| npx playwright install chromium | ||
| npm run test:e2e:ci |
There was a problem hiding this comment.
Add --with-deps to playwright install chromium for CI.
ubuntu-latest runners may not have all Chromium system dependencies pre-installed. Without --with-deps, the browser launch can fail with missing shared-library errors. The local run instructions in e2e/README.md (line 62) already include --with-deps.
Suggested fix
- npx playwright install chromium
+ npx playwright install --with-deps chromium🤖 Prompt for AI Agents
In @.github/workflows/portal-ui-e2e.yml around lines 78 - 82, Update the CI step
that calls the Playwright installer: change the command invocation "npx
playwright install chromium" in the run block to include the --with-deps flag
(i.e., "npx playwright install --with-deps chromium") so the installer also
installs required system dependencies for Chromium.
There was a problem hiding this comment.
Thanks for the suggestion. We are intentionally not applying this in this PR: CI has been stable and we chose to keep the current install mode for lower runtime. If dependency-related flakes appear, we will switch back to --with-deps.
There was a problem hiding this comment.
@nobodyiam, understood! Since your CI has been stable without the flag and you're optimizing for faster runs, keeping the current approach makes sense. If you do encounter any browser launch issues later, the --with-deps flag will be there as a fallback. Thanks for the clarification!
| import static org.junit.Assert.assertEquals; | ||
|
|
||
| import org.junit.Test; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use JUnit 5 instead of JUnit 4.
The test imports org.junit.Test and org.junit.Assert (JUnit 4). Per project guidelines, new tests should use JUnit 5.
♻️ Proposed fix
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;As per coding guidelines, **/src/test/java/**/*Test.java: "use JUnit 5 as the default framework".
📝 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.
| import static org.junit.Assert.assertEquals; | |
| import org.junit.Test; | |
| import static org.junit.jupiter.api.Assertions.assertEquals; | |
| import org.junit.jupiter.api.Test; |
🤖 Prompt for AI Agents
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SignInControllerTest.java`
around lines 19 - 21, The test SignInControllerTest is importing JUnit 4
(org.junit.Test and org.junit.Assert.assertEquals); update it to JUnit 5 by
replacing the imports with org.junit.jupiter.api.Test and
org.junit.jupiter.api.Assertions.assertEquals (or a static import of
org.junit.jupiter.api.Assertions.assertEquals) and remove any junit4-specific
imports/annotations so the class uses the JUnit 5 `@Test` and assertion APIs
consistently.
There was a problem hiding this comment.
Thanks for the suggestion. We are not applying this in this PR to keep scope focused on the Portal UI E2E gate and related fixes. Also, most existing controller tests in this module are still on JUnit 4, so we are keeping consistency here for now.
There was a problem hiding this comment.
@nobodyiam, understood! Keeping the PR focused on the E2E gate makes sense, and consistency with the existing test suite is a practical consideration. Thank you for the clarification.
🧠 Learnings used
Learnt from: CR
Repo: apolloconfig/apollo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-03T05:44:23.861Z
Learning: Applies to **/src/test/java/**/*Test.java : Put new tests under the module's `src/test/java` with `*Test` suffix and use JUnit 5 as the default framework
There was a problem hiding this comment.
Pull request overview
Adds a Portal UI E2E testing gate and expands Portal UI-focused automated coverage (Playwright E2E + portal controller unit tests) to reduce manual regression for common UI and Config Service flows.
Changes:
- Introduces a new Playwright-based Portal UI E2E suite (
e2e/portal-e2e) covering smoke, regression, high-priority user-guide, and Config Service full-chain scenarios. - Adds a GitHub Actions workflow to run Portal UI E2E on relevant PR changes, booting
apollo-assemblywith H2 and uploading artifacts on failure. - Expands unit test coverage for multiple Portal controllers used by UI paths.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/portal-ui-e2e.yml |
Adds PR-scoped CI job to boot assembly and run Portal UI Playwright E2E. |
e2e/portal-e2e/tests/portal-core.spec.js |
Smoke E2E: login, app creation, item lifecycle, rollback, release history assertions. |
e2e/portal-e2e/tests/portal-regression.spec.js |
Regression E2E: duplicate create rejection, cluster/namespace creation, export + instance-view reachability. |
e2e/portal-e2e/tests/portal-priority.spec.js |
High-priority E2E: role grant/revoke, text edit publish, public namespace link/override, grayscale publish/merge/discard. |
e2e/portal-e2e/tests/portal-configservice.spec.js |
Full-chain E2E validating Config Service /configs, /configfiles/raw, and /notifications/v2 behavior across publish/gray/rollback. |
e2e/portal-e2e/tests/helpers/portal-helpers.js |
Shared Playwright helpers for UI actions and Portal/ConfigService API polling utilities. |
e2e/portal-e2e/scripts/wait-for-ready.sh |
Readiness/warm-up script used by CI to ensure Portal/Admin/Config are usable before E2E. |
e2e/portal-e2e/playwright.config.js |
Playwright configuration (timeouts, retries/workers, reporters, baseURL). |
e2e/portal-e2e/package.json |
Adds Node package manifest and scripts to run the Playwright suite. |
e2e/portal-e2e/package-lock.json |
Locks Playwright dependency tree for CI/local consistency. |
e2e/portal-e2e/.gitignore |
Ignores Playwright and Node build artifacts under the new suite. |
e2e/README.md |
Documents E2E suites, local run commands, tags, and CI trigger paths. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ClusterControllerTest.java |
Adds controller test coverage for cluster create/load/delete behavior. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConfigsExportControllerTest.java |
Adds tests for export filename behavior and export endpoints. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ConfigsImportControllerTest.java |
Adds tests for import endpoints, env parsing, and conflict-action handling. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/EnvControllerTest.java |
Adds tests for active env list endpoint. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/InstanceControllerTest.java |
Adds tests for instance query endpoints and releaseId parsing/dedup. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/NamespaceLockControllerTest.java |
Adds tests for namespace lock and lock-info endpoints. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/OrganizationControllerTest.java |
Adds tests for organization loading endpoint. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PageSettingControllerTest.java |
Adds tests for page settings endpoint values. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PrefixPathControllerTest.java |
Adds tests for prefix-path behavior and fallback to servlet context path. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ReleaseHistoryControllerTest.java |
Adds tests for permission-gated release history responses. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SignInControllerTest.java |
Adds tests for signin/login page handler. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SsoHeartbeatControllerTest.java |
Adds tests ensuring SSO heartbeat delegates to handler. |
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SystemInfoControllerTest.java |
Adds tests for system-info environment/service discovery and health checks. |
CHANGES.md |
Records the addition of the Portal UI Playwright E2E CI gate. |
AGENTS.md |
Updates contributor guidelines around branching and CHANGES.md PR-link requirements. |
Files not reviewed (1)
- e2e/portal-e2e/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| cd e2e/portal-e2e | ||
| npm ci | ||
| npx playwright install chromium |
There was a problem hiding this comment.
The CI workflow installs Playwright’s Chromium browser without OS dependencies. On ubuntu runners this can lead to runtime failures (missing libraries), and it’s also inconsistent with the local-run docs which use --with-deps. Consider using the --with-deps chromium install mode (or the official Playwright GitHub Action) to reduce flakiness.
| npx playwright install chromium | |
| npx playwright install --with-deps chromium |
There was a problem hiding this comment.
Thanks for the suggestion. We are intentionally not applying this in this PR: CI has been stable and we chose to keep the current install mode for lower runtime. If dependency-related flakes appear, we will switch back to --with-deps.
| app_status="$(curl -sS -o /tmp/apollo-warmup-app.json -w '%{http_code}' \ | ||
| -b "$cookie_file" -H 'Content-Type: application/json' -X POST \ | ||
| "${PORTAL_URL}/apps" -d "$app_payload" || true)" | ||
|
|
||
| item_payload='{"key":"timeout","value":"100","comment":"warmup","lineNum":1}' | ||
| item_status="$(curl -sS -o /tmp/apollo-warmup-item.json -w '%{http_code}' \ | ||
| -b "$cookie_file" -H 'Content-Type: application/json' -X POST \ | ||
| "${PORTAL_URL}/apps/${app_id}/envs/LOCAL/clusters/default/namespaces/application/item" \ | ||
| -d "$item_payload" || true)" | ||
|
|
||
| release_payload='{"releaseTitle":"warmup-release","releaseComment":"warmup"}' | ||
| release_status="$(curl -sS -o /tmp/apollo-warmup-release.json -w '%{http_code}' \ |
There was a problem hiding this comment.
The readiness script writes warm-up responses to fixed paths under /tmp (e.g., /tmp/apollo-warmup-app.json). These files are never cleaned up and can cause noise/collisions on reused/self-hosted runners. Prefer writing to a mktemp file (and trap cleanup) or redirecting the body to /dev/null when only the status code is needed.
| app_status="$(curl -sS -o /tmp/apollo-warmup-app.json -w '%{http_code}' \ | |
| -b "$cookie_file" -H 'Content-Type: application/json' -X POST \ | |
| "${PORTAL_URL}/apps" -d "$app_payload" || true)" | |
| item_payload='{"key":"timeout","value":"100","comment":"warmup","lineNum":1}' | |
| item_status="$(curl -sS -o /tmp/apollo-warmup-item.json -w '%{http_code}' \ | |
| -b "$cookie_file" -H 'Content-Type: application/json' -X POST \ | |
| "${PORTAL_URL}/apps/${app_id}/envs/LOCAL/clusters/default/namespaces/application/item" \ | |
| -d "$item_payload" || true)" | |
| release_payload='{"releaseTitle":"warmup-release","releaseComment":"warmup"}' | |
| release_status="$(curl -sS -o /tmp/apollo-warmup-release.json -w '%{http_code}' \ | |
| app_status="$(curl -sS -o /dev/null -w '%{http_code}' \ | |
| -b "$cookie_file" -H 'Content-Type: application/json' -X POST \ | |
| "${PORTAL_URL}/apps" -d "$app_payload" || true)" | |
| item_payload='{"key":"timeout","value":"100","comment":"warmup","lineNum":1}' | |
| item_status="$(curl -sS -o /dev/null -w '%{http_code}' \ | |
| -b "$cookie_file" -H 'Content-Type: application/json' -X POST \ | |
| "${PORTAL_URL}/apps/${app_id}/envs/LOCAL/clusters/default/namespaces/application/item" \ | |
| -d "$item_payload" || true)" | |
| release_payload='{"releaseTitle":"warmup-release","releaseComment":"warmup"}' | |
| release_status="$(curl -sS -o /dev/null -w '%{http_code}' \ |
There was a problem hiding this comment.
Fixed in commit 3d8fc37. Thanks for the suggestion.
| const USERNAME = 'apollo'; | ||
| const PASSWORD = 'admin'; |
There was a problem hiding this comment.
E2E helpers currently hardcode the Portal UI credentials (apollo / admin). Even if these are default dev creds, keeping them inline makes it harder to run against custom setups and increases the risk of accidentally reusing real credentials. Consider reading USERNAME/PASSWORD from environment variables with safe defaults.
| const USERNAME = 'apollo'; | |
| const PASSWORD = 'admin'; | |
| const USERNAME = process.env.PORTAL_USERNAME || 'apollo'; | |
| const PASSWORD = process.env.PORTAL_PASSWORD || 'admin'; |
There was a problem hiding this comment.
Fixed in commit 3d8fc37. Thanks for the suggestion.
| curl -fsS -c "$cookie_file" -b "$cookie_file" \ | ||
| -H 'Content-Type: application/x-www-form-urlencoded' \ | ||
| -X POST "${PORTAL_URL}/signin" \ | ||
| --data 'username=apollo&password=admin' >/dev/null 2>&1 || { |
There was a problem hiding this comment.
The readiness script posts hardcoded credentials to /signin. For flexibility (and to avoid embedding credentials in repo scripts), consider sourcing username/password from environment variables with defaults matching the dev setup.
There was a problem hiding this comment.
Fixed in commit 3d8fc37. Thanks for the suggestion.
|
|
||
| async function waitForApiResponse(page, method, urlFragment, status = DEFAULT_SUCCESS_STATUSES) { | ||
| return page.waitForResponse( | ||
| (response) => |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
Too many errors. (9% scanned).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return fallbackPanel; | ||
| } | ||
|
|
||
| async function waitForApiResponse(page, method, urlFragment, status = DEFAULT_SUCCESS_STATUSES) { |
There was a problem hiding this comment.
'default parameters' is only available in ES6 (use 'esversion: 6').
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| const fallbackPanel = page.locator('.panel.namespace-panel:not(.hidden)').filter({ | ||
| has: page.locator('b.namespace-name', { hasText: candidates[0] || DEFAULT_NAMESPACE }), | ||
| }).first(); | ||
| await fallbackPanel.waitFor({ state: 'visible', timeout: 30000 }); |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
| } | ||
|
|
||
| const fallbackPanel = page.locator('.panel.namespace-panel:not(.hidden)').filter({ |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| }).first(); | ||
|
|
||
| if (await panel.count()) { | ||
| await panel.waitFor({ state: 'visible', timeout: 30000 }); |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return new Promise((resolve) => setTimeout(resolve, milliseconds)); | ||
| } | ||
|
|
||
| async function locateNamespacePanel(page, namespaceName) { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
|
|
||
| async function sleep(milliseconds) { | ||
| return new Promise((resolve) => setTimeout(resolve, milliseconds)); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return Array.from(new Set([namespaceName, stripped].filter(Boolean))); | ||
| } | ||
|
|
||
| async function sleep(milliseconds) { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return [DEFAULT_NAMESPACE]; | ||
| } | ||
|
|
||
| const stripped = namespaceName.replace(/\.(properties|yaml|yml|json)$/i, ''); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
|
|
||
| function normalizeNotificationNamespace(namespaceName) { | ||
| return `${namespaceName || ''}`.replace(/\.properties$/i, '').toLowerCase(); |
There was a problem hiding this comment.
'template literal syntax' is only available in ES6 (use 'esversion: 6').
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
|
|
||
| async function waitForApiResponse(page, method, urlFragment, status = DEFAULT_SUCCESS_STATUSES) { | ||
| return page.waitForResponse( | ||
| (response) => |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
Too many errors. (9% scanned).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return fallbackPanel; | ||
| } | ||
|
|
||
| async function waitForApiResponse(page, method, urlFragment, status = DEFAULT_SUCCESS_STATUSES) { |
There was a problem hiding this comment.
'default parameters' is only available in ES6 (use 'esversion: 6').
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| const fallbackPanel = page.locator('.panel.namespace-panel:not(.hidden)').filter({ | ||
| has: page.locator('b.namespace-name', { hasText: candidates[0] || DEFAULT_NAMESPACE }), | ||
| }).first(); | ||
| await fallbackPanel.waitFor({ state: 'visible', timeout: 30000 }); |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
| } | ||
|
|
||
| const fallbackPanel = page.locator('.panel.namespace-panel:not(.hidden)').filter({ |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| }).first(); | ||
|
|
||
| if (await panel.count()) { | ||
| await panel.waitFor({ state: 'visible', timeout: 30000 }); |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return new Promise((resolve) => setTimeout(resolve, milliseconds)); | ||
| } | ||
|
|
||
| async function locateNamespacePanel(page, namespaceName) { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
|
|
||
| async function sleep(milliseconds) { | ||
| return new Promise((resolve) => setTimeout(resolve, milliseconds)); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return Array.from(new Set([namespaceName, stripped].filter(Boolean))); | ||
| } | ||
|
|
||
| async function sleep(milliseconds) { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return [DEFAULT_NAMESPACE]; | ||
| } | ||
|
|
||
| const stripped = namespaceName.replace(/\.(properties|yaml|yml|json)$/i, ''); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
|
|
||
| function normalizeNotificationNamespace(namespaceName) { | ||
| return `${namespaceName || ''}`.replace(/\.properties$/i, '').toLowerCase(); |
There was a problem hiding this comment.
'template literal syntax' is only available in ES6 (use 'esversion: 6').
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
|
|
||
| async function waitForApiResponse(page, method, urlFragment, status = DEFAULT_SUCCESS_STATUSES) { | ||
| return page.waitForResponse( | ||
| (response) => |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
Too many errors. (9% scanned).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return fallbackPanel; | ||
| } | ||
|
|
||
| async function waitForApiResponse(page, method, urlFragment, status = DEFAULT_SUCCESS_STATUSES) { |
There was a problem hiding this comment.
'default parameters' is only available in ES6 (use 'esversion: 6').
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| const fallbackPanel = page.locator('.panel.namespace-panel:not(.hidden)').filter({ | ||
| has: page.locator('b.namespace-name', { hasText: candidates[0] || DEFAULT_NAMESPACE }), | ||
| }).first(); | ||
| await fallbackPanel.waitFor({ state: 'visible', timeout: 30000 }); |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
| } | ||
|
|
||
| const fallbackPanel = page.locator('.panel.namespace-panel:not(.hidden)').filter({ |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| }).first(); | ||
|
|
||
| if (await panel.count()) { | ||
| await panel.waitFor({ state: 'visible', timeout: 30000 }); |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return new Promise((resolve) => setTimeout(resolve, milliseconds)); | ||
| } | ||
|
|
||
| async function locateNamespacePanel(page, namespaceName) { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
|
|
||
| async function sleep(milliseconds) { | ||
| return new Promise((resolve) => setTimeout(resolve, milliseconds)); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return Array.from(new Set([namespaceName, stripped].filter(Boolean))); | ||
| } | ||
|
|
||
| async function sleep(milliseconds) { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| return [DEFAULT_NAMESPACE]; | ||
| } | ||
|
|
||
| const stripped = namespaceName.replace(/\.(properties|yaml|yml|json)$/i, ''); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
| } | ||
|
|
||
| function normalizeNotificationNamespace(namespaceName) { | ||
| return `${namespaceName || ''}`.replace(/\.properties$/i, '').toLowerCase(); |
There was a problem hiding this comment.
'template literal syntax' is only available in ES6 (use 'esversion: 6').
There was a problem hiding this comment.
Thanks for the report. We are not changing this code based on this comment because this is a tooling false positive from Hound's JS parser/ruleset (ES5-level) on modern Node/Playwright syntax. These files are intentionally ES6+, and the PR checks pass under Node 20. We will follow up by tuning lint/review rules for this e2e path.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@e2e/portal-e2e/tests/helpers/portal-helpers.js`:
- Around line 1245-1261: In rollbackLatestRelease, destructure env (e.g., const
{ namespaceName = DEFAULT_NAMESPACE, env = DEFAULT_ENV } = options) and replace
the hardcoded '/envs/LOCAL/releases/' in the waitForApiResponse call with an
interpolated path that uses the env variable (e.g., `/envs/${env}/releases/`) so
the API matcher behaves consistently with other helpers; update the
waitForApiResponse invocation in the rollbackLatestRelease function accordingly.
🧹 Nitpick comments (3)
e2e/portal-e2e/scripts/wait-for-ready.sh (1)
39-52: Consider atrapto clean up the cookie file on unexpected exit.If the script is interrupted (e.g., CI timeout/SIGTERM) between
mktempon line 42 andrmon line 75, the temp file leaks. A local trap makes cleanup unconditional:🧹 Proposed fix
warm_up_portal_admin_path() { local app_id cookie_file app_payload item_payload release_payload app_status item_status release_status cookie_file="$(mktemp)" + trap 'rm -f "$cookie_file"' RETURN app_id="warmup$(date +%s)$RANDOM" curl -fsS -c "$cookie_file" -b "$cookie_file" \ -H 'Content-Type: application/x-www-form-urlencoded' \ -X POST "${PORTAL_URL}/signin" \ --data-urlencode "username=${PORTAL_USERNAME}" \ --data-urlencode "password=${PORTAL_PASSWORD}" >/dev/null 2>&1 || { - rm -f "$cookie_file" return 1 }With
trap ... RETURN, the cookie file is removed whenever the function returns (success or failure), and you can drop the explicitrm -fon lines 50 and 75.e2e/portal-e2e/tests/helpers/portal-helpers.js (2)
188-193: Unescaped value interpolated into jQuery attribute selector.
first.idis spliced directly intooption[value="${first.id}"]. If the ID ever contains"or], the selector breaks. The same pattern recurs at lines 364 and 617.For E2E tests with controlled data this is low risk, but a helper like
CSS.escape(first.id)(or a simple quote-escaping utility) would make these selectors robust.
680-686: Redundantawait—createBranchResponseis already settled afterPromise.all.
Promise.allon line 680 already resolvescreateBranchResponse. Line 685 re-awaits the same promise, which is a no-op. You can destructure fromPromise.allinstead:Suggested simplification
- await Promise.all([ - createBranchResponse, + const [response] = await Promise.all([ + createBranchResponse, page.locator('#createBranchTips .modal-footer button.btn-primary').first().click(), ]); - const response = await createBranchResponse; const body = await response.json().catch(() => null);
|
For transparency: this PR was delivered end-to-end by Codex, including solution planning, implementation, test design/execution, CI gate setup/optimization, and PR preparation/follow-up. |
What's the purpose of this PR
This PR adds a Portal UI testing gate and expands Portal UI-oriented test coverage so that common UI flows can be validated automatically in PR checks, reducing manual local regression.
Which issue(s) this PR fixes:
N/A
Brief changelog
Add a new GitHub Actions workflow for Portal UI E2E:
.github/workflows/portal-ui-e2e.ymlapollo-portal,apollo-assembly,e2e/portal-e2e,scripts/sql, and workflow file itself.apollo-assemblywith H2, runs Playwright tests, uploads report/trace/log artifacts on failure.Add Playwright-based Portal E2E suite:
e2e/portal-e2e/configs/**,/configfiles/**, and/notifications/v2behavior after publish/gray/rollback.Add backend Portal controller tests for additional UI-facing endpoints:
ClusterControllerTestConfigsExportControllerTestConfigsImportControllerTestEnvControllerTestInstanceControllerTestNamespaceLockControllerTestOrganizationControllerTestPageSettingControllerTestPrefixPathControllerTestReleaseHistoryControllerTestSignInControllerTestSsoHeartbeatControllerTestSystemInfoControllerTestUpdate docs/changelog:
e2e/README.mdCHANGES.mdFollow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.mvn spotless:applyto format your code.CHANGESlog.Local validation
./mvnw -B -pl apollo-portal -am test(pass)Summary by CodeRabbit
Tests
Chores
Documentation