Testing: Add team allocation phase module e2e coverage#1914
Open
mathildeshagl wants to merge 10 commits into
Open
Testing: Add team allocation phase module e2e coverage#1914mathildeshagl wants to merge 10 commits into
Testing: Add team allocation phase module e2e coverage#1914mathildeshagl wants to merge 10 commits into
Conversation
Add the team allocation server, client remote, and own Postgres to the e2e Docker stack following the phase-module blueprint: compose services, DB_TEAM_ALLOCATION_* env, same-origin host var, nginx proxy locations, and a /team-allocation/api/info readiness poll in global-setup.
Seed standalone Team Allocation phases for the lecturer journey (Stan + Selma) and student journey (Stan), plus a TestCourse negative-auth phase, and export their phase-id constants. Isolated from the graph Team Allocation phase so parallel Playwright files never clobber allocations.
Add TeamAllocationPage with locators for the allocations, survey, settings, and participants views, and a module-federation smoke test asserting the remote renders inside the core shell.
Lecturer journey creates a team through the settings UI, publishes the allocation via the TEASE save endpoint (the external tool's callback), and verifies the result on the allocations + participants views and the API. Student journey opens the survey remote and reads its allocated team via the allocation API (the module has no student-facing team view).
Assert the phase endpoints reject unauthenticated and wrong-role requests (401) and accept an enrolled student on the read endpoints.
Add the module to the stack diagram and intro, and note the TEASE-external allocation flow and the standalone journey phases.
…cation read Open the survey timeframe in the student journey's setup so the survey remote renders its form instead of the "not configured" error page, and read the allocated team from the single-allocation endpoint, which returns the team UUID as a bare string rather than a participation object.
5 tasks
Contributor
📝 WalkthroughWalkthroughThe E2E stack now runs Team Allocation services with dedicated fixtures, routes module traffic through the core client, waits for readiness, and adds Playwright coverage for API authorization, lecturer allocation, student allocation, and module-federation loading. ChangesTeam allocation E2E coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant E2E_Runner
participant client_core
participant Team_Allocation_Server
participant Team_Allocation_Client
E2E_Runner->>Team_Allocation_Client: wait for healthy
E2E_Runner->>client_core: poll /team-allocation/api/info
client_core->>Team_Allocation_Server: proxy API requests
client_core->>Team_Allocation_Client: proxy module requests
E2E_Runner->>Team_Allocation_Client: run Playwright journeys
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/src/pages/TeamAllocationPage.ts (1)
41-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider scoping allocation assertions to their container for precision.
expectAllocatedMemberchecks the member name on the entire page (this.page.getByText(memberFullName)), not within the team card.expectParticipantAllocatedTeamcheckslastNameandteamNameindependently with.first()rather than verifying they co-occur on the same table row. Both work correctly with the current single-team, single-participant e2e fixtures, but would produce false positives if the seed data grows. Scoping the member check to the team card container and asserting row-level co-occurrence would make these assertions more robust.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/src/pages/TeamAllocationPage.ts` around lines 41 - 53, Scope expectAllocatedMember’s member lookup to the team card container identified by teamCardHeading(teamName). For expectParticipantAllocatedTeam, locate the participant row containing lastName within `#table-view`, then assert teamName within that same row instead of using independent first() lookups. Preserve the existing visibility checks and timeouts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/src/pages/TeamAllocationPage.ts`:
- Around line 41-53: Scope expectAllocatedMember’s member lookup to the team
card container identified by teamCardHeading(teamName). For
expectParticipantAllocatedTeam, locate the participant row containing lastName
within `#table-view`, then assert teamName within that same row instead of using
independent first() lookups. Preserve the existing visibility checks and
timeouts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21a17fa8-b843-4ad5-91c6-73545d914070
📒 Files selected for processing (14)
docker-compose.e2e.ymle2e/.env.e2ee2e/README.mde2e/nginx/client-core.confe2e/seed/e2e_seed.sqle2e/src/data/constants.tse2e/src/env.tse2e/src/global-setup.tse2e/src/pages/TeamAllocationPage.tse2e/tests/api/team-allocation.api.spec.tse2e/tests/team-allocation/helpers.tse2e/tests/team-allocation/lecturer-journey.spec.tse2e/tests/team-allocation/mf-smoke.spec.tse2e/tests/team-allocation/student-journey.spec.ts
Correct the deleteTeamByName doc comment (callers clear allocations first; the helper does not), make the internal-only URL builders module-private, and add the certificate module to the README stack diagram, intro, and blueprint list.
…ion-phase-module-coverage # Conflicts: # e2e/README.md
mathildeshagl
had a problem deploying
to
prompt-dev-vm
GitHub Actions
Error
…ion-phase-module-coverage # Conflicts: # e2e/.env.e2e
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/.env.e2e`:
- Line 101: Move the EXAMPLE_HOST entry in the environment configuration so it
appears before INTERVIEW_HOST, preserving its existing value and all other
entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a114561-29c4-4116-becf-5dd7670b4caa
📒 Files selected for processing (3)
docker-compose.e2e.ymle2e/.env.e2ee2e/seed/e2e_seed.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose.e2e.yml
- e2e/seed/e2e_seed.sql
| INTRO_COURSE_HOST=http://localhost:8082 | ||
| DEVOPS_CHALLENGE_HOST=http://localhost:9010 | ||
| TEAM_ALLOCATION_HOST=http://localhost:8083 | ||
| EXAMPLE_HOST=http://localhost:8086 |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reorder EXAMPLE_HOST before INTERVIEW_HOST.
This currently triggers the reported dotenv-linter UnorderedKey warning and may fail environments that enforce dotenv linting.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 101-101: [UnorderedKey] The EXAMPLE_HOST key should go before the INTERVIEW_HOST key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/.env.e2e` at line 101, Move the EXAMPLE_HOST entry in the environment
configuration so it appears before INTERVIEW_HOST, preserving its existing value
and all other entries.
Source: Linters/SAST tools
mathildeshagl
requested a deployment
to
prompt-dev-vm
GitHub Actions
Waiting
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
1 participant
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.
📝 Pull Request Template
Use this template to provide all necessary information for reviewing and testing your changes.
✨ What is the change?
Adds end-to-end coverage for the team allocation phase module and boots it in the
make test-e2estack, following the phase-module blueprint proven for self team allocation, assessment, interview, and certificate.server-team-allocation+client-team-allocation+ a dedicateddb-team-allocationindocker-compose.e2e.yml,DB_TEAM_ALLOCATION_*env with a same-origin host var,/team-allocation/api/+/team-allocation/nginx proxy locations, and a/team-allocation/api/inforeadiness poll inglobal-setup.e2e/tests/team-allocation/,e2e/tests/api/team-allocation.api.spec.ts,e2e/src/pages/TeamAllocationPage.ts):e2e/README.mdupdated (stack diagram, intro, and the TEASE-external allocation note).Team allocation's algorithm runs in the external TEASE tool, so the lecturer journey publishes the computed assignment through the phase server's TEASE save endpoint (the tool's callback) rather than clicking a matchmaking button. The module has no student-facing "my team" page, so the student reads their allocated team through the allocation API.
📌 Reason for the change / Link to issue
The team allocation service (
servers/team_allocation) and client (clients/team_allocation_component) had no e2e coverage and were not part of the e2e stack, leaving Module Federation remote loading, cross-service prompt-sdk auth, and the per-service database pattern unexercised for this module.Closes #1818
🧪 How to Test
make test-e2e.team-allocation ready (via proxy)).tests/team-allocation/{mf-smoke,lecturer-journey,student-journey}.spec.tsandtests/api/team-allocation.api.spec.ts(this branch: 146 passed).make test-e2e-ui(Playwright UI athttp://127.0.0.1:8123), then run the team allocation specs;make test-e2e-downto tear down.🖼️ Screenshots (if UI changes are included)
No product UI changes — this PR adds tests, e2e stack wiring, and seed data only.
✅ PR Checklist
Summary by CodeRabbit
New Features
/team-allocation, and additional startup readiness gates.Tests
Documentation