-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(system-e2e): Move system test ownership to relevant teams #16479
Conversation
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 98 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
WalkthroughThe changes made in this pull request involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant TeamA as @island-is/aranja
participant TeamB as @island-is/norda-applications
participant TeamC as @island-is/deloitte
participant TeamD as @island-is/advania
participant TeamE as @island-is/hugsmidjan
participant TeamF as @island-is/juni
participant TeamG as @island-is/stefna
participant TeamH as @island-is/kolibri-justice-league
TeamA->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/admin-portal/
TeamB->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/application-system/
TeamC->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/application-system/acceptance/parental-leave.spec.ts
TeamD->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/consultation-portal/
TeamE->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/service-portal/
TeamF->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/service-portal/
TeamG->>SystemTests: Owns /apps/system-e2e/src/tests/islandis/service-portal/
TeamH->>SystemTests: Owns /apps/system-e2e/src/tests/judicial-system/
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/CODEOWNERS (2)
389-390
: Fix typo in section headerThe "Catch-all" section is a good addition, but there's a typo in the comment. Change "Cach-all" to "Catch-all".
The ownership assignment for
/apps/system-e2e/
to@island-is/qa
is appropriate as a fallback for all system tests.-# Cach-all +# Catch-all
391-399
: LGTM: Appropriate ownership assignments for system testsThe ownership assignments for specific system test directories are logical and well-structured. They distribute responsibility to relevant teams, which should improve accountability and efficiency in maintaining the tests.
For consistency, consider adding a comment to separate the Judicial system tests, similar to the "Islandis" comment:
/apps/system-e2e/src/tests/islandis/web/ @island-is/hugsmidjan @island-is/juni @island-is/stefna + +# Judicial system /apps/system-e2e/src/tests/judicial-system/ @island-is/kolibri-justice-leagueThis minor change would improve the overall organization of the file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/CODEOWNERS (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
.github/CODEOWNERS (1)
388-388
: LGTM: Clear section header addedThe addition of the "System tests" section header improves the organization and readability of the CODEOWNERS file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/CODEOWNERS (1)
390-391
: Remove or relocate the "# Catch-all" commentThe "# Catch-all" comment on line 390 appears to be misplaced as it's not followed by an actual catch-all rule. Consider removing this comment or relocating it to a more appropriate section if there's a catch-all rule elsewhere in the file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/CODEOWNERS (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.github/CODEOWNERS (2)
388-399
: LGTM: Improved organization of system test ownershipThe new ownership assignments for system tests are well-structured and provide more granular control. This change improves the clarity of ownership and responsibility for different parts of the system tests.
386-399
: LGTM: Well-structured ownership assignmentsThe new system test ownership assignments are well-organized with clear categories. This structure improves readability and makes it easier to locate and understand specific assignments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Move system test ownership to relevant teams * Update CODEOWNERS * Update .github/CODEOWNERS --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Teams should own their tests fully.
Summary by CodeRabbit
/apps/system-e2e/
directory.