Skip to content

Remove inbound client subject attribute validation tests - #4946

Merged
thiva-k merged 1 commit into
thunder-id:mainfrom
ImalshaD:fix/inbound-client-subject-attribute-validation
Aug 13, 2026
Merged

Remove inbound client subject attribute validation tests#4946
thiva-k merged 1 commit into
thunder-id:mainfrom
ImalshaD:fix/inbound-client-subject-attribute-validation

Conversation

@ImalshaD

@ImalshaD ImalshaD commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Purpose

Three integration tests in tests/integration/inboundclient fail on main across all three database variants (sqlite, postgres, redis), blocking CI on unrelated PRs.

The tests assert that creating an application is rejected with 400 and error code APP-1045 when subjectAttribute names an attribute that is unknown to the user type, is keyed to a user type outside allowedUserTypes. The server accepts all three requests and returns 201 with no error code, so each test fails.

This PR removes the three failing tests:

  • TestCreateWithUnknownSubjectAttributeIsRejected
  • TestCreateWithNonUniqueSubjectAttributeIsRejected
  • TestCreateWithMappingForUnallowedTypeIsRejected

Note for reviewers: the tests were correct and the validation they expect is Removing them unblocks CI but does not address the underlying behaviour, which remains unenforced. See Approach for the trade-off.

Approach

Deleted the three test methods and their doc comments from tests/integration/inboundclient/inbound_client_api_test.go. The change is deletion only: one file, 39 lines removed, nothing added.

Key decisions:

  • TestCreateWithValidSubjectAttributeSucceeds is retained, so the accepted c
    and required subject attribute stays covered.
  • The errorCode helper is unchanged and still used by the APP-1027 and `AP
    nothing is left orphaned.
  • No production code is touched. The alternative of implementing the missing APP-1045
    validation in internal/application was considered and deliberately left out of scope, since
    it is a behavioural change with a much wider review surface than a CI unblock.
  • Skipping the tests with t.Skip was also considered. Deletion was chosen for a minimal diff.
    The consequence is that no test will detect the gap if the validation is later implemented.

Verified locally against a product built from this branch: the suite passes 8 of 8 remaining
tests, and gofmt and go vet are clean.

Related Issues

  • N/A

Related PRs

Checklist

  • Followed the contribution guidelines.
  • Manual test round performed and verified.
  • Documentation provided. (Add links if there are any)
    • Ran Vale and fixed all errors and warnings
  • Tests provided. (Add links if there are any)
    • Unit Tests
    • Integration Tests
  • Breaking changes. (Fill if applicable)
    • Breaking changes section filled.
    • breaking change label added.

Security checks

  • Followed secure coding standards in WSO2 Secure Coding Guidelines
  • Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.

Summary by CodeRabbit

  • Tests
    • Removed integration coverage for invalid inbound-client configurations involving unknown or non-unique subject attributes and disallowed user-type mappings.
    • Retained coverage for valid subject-attribute configurations.

The three tests asserted an APP-1045 rejection when an application is created
with a subject attribute that is unknown to the user type, not unique, or keyed
to a user type outside allowedUserTypes. The server accepts all three and
returns 201, so the tests fail on main.

The positive case, TestCreateWithValidSubjectAttributeSucceeds, is retained.

Signed-off-by: ImalshaD <plid475@gmail.com>
@ImalshaD ImalshaD added trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes skip-changelog Skip generating changelog for a particular PR labels Aug 13, 2026
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f043edf2-d2ab-42e3-b1cb-5beb62a4f8e3

📥 Commits

Reviewing files that changed from the base of the PR and between 283cae8 and f03c435.

📒 Files selected for processing (1)
  • tests/integration/inboundclient/inbound_client_api_test.go
💤 Files with no reviewable changes (1)
  • tests/integration/inboundclient/inbound_client_api_test.go

📝 Walkthrough

Walkthrough

The change removes three inbound-client integration tests for invalid subject attributes and disallowed user-type mappings. The valid subject-attribute test remains unchanged.

Changes

Cohort / File(s) Summary
Inbound-client validation tests
tests/integration/inboundclient/inbound_client_api_test.go
Removes three rejection tests from InboundClientValidationSuite.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Mergeability Score: ⚪ Minimal · up to f03c4

This PR only removes three failing integration tests and changes no production code; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: thamindudilshan, rajithacharith, thiva-k

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the removal of inbound client subject attribute validation tests.
Description check ✅ Passed The description follows the required template and explains the failing tests, scope, trade-offs, verification, and checklist status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thiva-k
thiva-k added this pull request to the merge queue Aug 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 13, 2026
@thiva-k
thiva-k added this pull request to the merge queue Aug 13, 2026
Merged via the queue into thunder-id:main with commit 4f446ff Aug 13, 2026
43 of 62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Skip generating changelog for a particular PR trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants