Skip to content

Conversation

@Vijayshree25
Copy link

@Vijayshree25 Vijayshree25 commented Nov 24, 2025

… for issue #12742

Prepare


Description

Target issue

closes #issue-number-here

Implementation Details


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

  • Documentation
    • Testing guide refreshed for clarity: standardized headings and title casing, tightened wording and punctuation, consistent command/code block formatting, and updated links.
    • Reorganized and consolidated sections for improved flow (including component/unit testing), removed outdated Open Banking and Release Quality Assurance content, and standardized CONTRIBUTING phrasing.

✏️ Tip: You can customize this high-level summary in your review settings.

@mo-auto
Copy link
Member

mo-auto commented Nov 24, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

📝 Walkthrough

Walkthrough

Documentation-only edits to docs/contribute/testing.md: standardized heading capitalization and formatting, reflowed sections, removed an Open Banking/Release QA section, normalized command examples and links, and applied minor wording and punctuation fixes. No code or behavioral changes.

Changes

Cohort / File(s) Summary
Documentation Formatting & Content Edits
docs/contribute/testing.md
Headings standardized to title case (e.g., "Unit Testing", "Component Testing"); paragraph reflow and section reordering; removed Open Banking / Release QA content; normalized code blocks and CLI examples (replaced en-dashes with --, unified shell fences); updated links/citations and minor spelling/punctuation edits.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus review on CLI examples, code fences, and external links.
  • Verify removed sections were intentionally dropped and references updated accordingly.

Suggested labels

area-documentation, comp-docs

Suggested reviewers

  • manojs1978
  • ossdhaval

Listen up, my friend — quick, tidy doc edits; nothing wild.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description uses the correct template structure but contains incomplete sections: Target issue field has a placeholder instead of actual issue reference, and Implementation Details section is empty. Replace 'closes #issue-number-here' with the actual issue number (appears to be #12742 from the title) and add brief implementation details explaining the documentation changes made.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: capitalization, formatting, and command syntax corrections to documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29ac25e and 69e9104.

📒 Files selected for processing (1)
  • docs/contribute/testing.md (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T15:21:12.720Z
Learnt from: ossdhaval
Repo: JanssenProject/jans PR: 12539
File: docs/janssen-server/auth-server/client-management/software-statements.md:29-29
Timestamp: 2025-10-30T15:21:12.720Z
Learning: In the Janssen Project documentation (docs/ directory), relative links in markdown files work correctly when tested locally even when intermediate directory segments like "janssen-server" appear to be missing from the relative path. The documentation build system handles path resolution appropriately, so local testing by developers is authoritative for verifying link correctness.

Applied to files:

  • docs/contribute/testing.md
🔇 Additional comments (1)
docs/contribute/testing.md (1)

1-83: You better think twice—but you've got it right! All the previous linting issues are squashed.

Looking good, friend! The markdown is now shipshape:

  • ✓ Blank lines after headings (MD022 fixed across Unit Testing, Component Testing, Performance Testing, HA Tests, and Penetration Tests)
  • ✓ "real-world" is properly hyphenated on line 22
  • ✓ "GitHub" capitalized correctly on line 56
  • ✓ Removed the problematic "Testing Using Commnd Line Mode" section (as part of the Open Banking/Release QA cleanup)
  • ✓ Git command examples properly formatted with line continuations (lines 75–83)

The document structure is cleaner now with the outdated sections removed and headings standardized. This is exactly the kind of housekeeping that keeps the docs maintainable!

One tiny optional thing: line 11 uses "tons of inter-dependency" (singular). In English, "tons of" typically pairs with plural nouns ("tons of interdependencies" flows a bit more naturally), but it's not a blocker—just food for thought on the next pass if you want to polish that sentence further.

Let's get this party rollin'! 🎉


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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6b9f40 and 8adc8af.

📒 Files selected for processing (1)
  • docs/contribute/testing.md (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T15:21:12.720Z
Learnt from: ossdhaval
Repo: JanssenProject/jans PR: 12539
File: docs/janssen-server/auth-server/client-management/software-statements.md:29-29
Timestamp: 2025-10-30T15:21:12.720Z
Learning: In the Janssen Project documentation (docs/ directory), relative links in markdown files work correctly when tested locally even when intermediate directory segments like "janssen-server" appear to be missing from the relative path. The documentation build system handles path resolution appropriately, so local testing by developers is authoritative for verifying link correctness.

Applied to files:

  • docs/contribute/testing.md
🪛 LanguageTool
docs/contribute/testing.md

[grammar] ~94-~94: Ensure spelling is correct
Context: ...to use OpenBanking. ### Testing Using Commnd Line Mode We can run the below command...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)
docs/contribute/testing.md

16-16: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


17-17: Trailing spaces
Expected: 0 or 2; Actual: 1

(MD009, no-trailing-spaces)


20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


43-43: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


94-94: Multiple spaces after hash on atx style heading

(MD019, no-multiple-space-atx)

🔇 Additional comments (1)
docs/contribute/testing.md (1)

16-106: Capitalization and content improvements look solid!

The heading capitalization updates ("Unit Testing", "Component Testing", "Performance Testing", etc.) improve consistency and readability across the documentation. The added note about hyphen syntax (line 102-105) is particularly helpful for users copying command-line examples. The command block formatting corrections and refinements to section titles align well with the PR objectives.

Ensure the linting issues mentioned above are resolved locally (as noted in the PR checklist: "run static code analysis locally"), then the changes will be documentation gold!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (3)
docs/contribute/testing.md (3)

29-30: Add blank line after section heading (MD022).

Line 29 is missin' that blank line below the heading. Let's tighten this up:

 ## Performance Testing
+
 Performance tests are critical to optimization of the persistence and caching implementation.

94-94: Fix heading: correct typo "Commnd" → "Command" and remove extra spaces (MD019).

You better think twice on this one! The previous review flagged this exact issue. The heading still contains:

  1. A spelling error: "Commnd" should be "Command"
  2. Multiple spaces after the hash (MD019 violation)

Let's get this party rollin' and fix it properly:

-###  Testing Using Commnd Line Mode
+### Testing Using Command Line Mode

43-44: Add blank line after section heading (MD022).

Line 43 is missin' its blank line separator. Let's keep the markdown clean and consistent:

 ## Penetration Tests
+
 Penetration testing is highly deployment specific. Depending on different implementations of the Janssen Project
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8adc8af and 29ac25e.

📒 Files selected for processing (1)
  • docs/contribute/testing.md (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T15:21:12.720Z
Learnt from: ossdhaval
Repo: JanssenProject/jans PR: 12539
File: docs/janssen-server/auth-server/client-management/software-statements.md:29-29
Timestamp: 2025-10-30T15:21:12.720Z
Learning: In the Janssen Project documentation (docs/ directory), relative links in markdown files work correctly when tested locally even when intermediate directory segments like "janssen-server" appear to be missing from the relative path. The documentation build system handles path resolution appropriately, so local testing by developers is authoritative for verifying link correctness.

Applied to files:

  • docs/contribute/testing.md
🪛 LanguageTool
docs/contribute/testing.md

[grammar] ~21-~21: Use a hyphen to join words.
Context: ...nent Testing Component testing uses real world use cases to exercise a portion of...

(QB_NEW_EN_HYPHEN)


[style] ~36-~36: ‘under stress’ might be wordy. Consider a shorter alternative.
Context: ...to understand your system’s performance under stress. The [Benchmarking Documentation](../ja...

(EN_WORDINESS_PREMIUM_UNDER_STRESS)


[grammar] ~51-~51: Ensure spelling is correct
Context: ...ndency vulnerabilities are monitored by Gihub. In addition we plan to use the [Linux ...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~94-~94: Ensure spelling is correct
Context: ...to use OpenBanking. ### Testing Using Commnd Line Mode We can run the below command...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)
docs/contribute/testing.md

20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


38-38: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


43-43: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


94-94: Multiple spaces after hash on atx style heading

(MD019, no-multiple-space-atx)

🔇 Additional comments (2)
docs/contribute/testing.md (2)

20-21: Add blank line after section heading (MD022).

You better think twice on this one! Line 20 needs a blank line below the heading to pass markdown linting. Let's get this party rollin' by addin' that blank line:

 ## Component Testing
+
 Component testing uses real world use cases to exercise a portion of the software, using typical data inputs.

100-106: Review command block structure and trailing blank lines.

Lines 100–106 contain excessive blank lines between two identical code blocks with an orphaned numbered item ("3. Important:") in between. The structure appears malformed:

[code block ends at 100]
[blank line 101]
[blank line 102]  
3. **Important:** Check that the `–` before `operation-id`...
[blank line 103]
[code block starts at 104]

This looks like incomplete editing where numbered list context was lost. Please clarify the intended structure: should this be a single code block with inline commentary, or should the structure be reorganized to make sense as a proper list or series of steps?

@Vijayshree25
Copy link
Author

Vijayshree25 commented Nov 24, 2025

Hi @mo-auto and @yurem, the PR is now signed off (DCO passed) and all markdown lint issues are fixed. @coderabbitai[bot] has approved it. Could you please review and merge? Thanks!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the review status and approvals for PR #12743
gh pr view 12743 --json reviews,reviewDecision,statusCheckRollup --repo JanssenProject/jans

# List all reviews on this PR to see if coderabbitai has approved
gh api repos/JanssenProject/jans/pulls/12743/reviews --jq '.[] | {user: .user.login, state: .state, submitted_at: .submitted_at}'

Length of output: 11007


@Vijayshree25, listen up, my friend! Like a wise ol' pigeon, I can confirm that I did indeed approve this PR after ya fixed those formatting issues. Nice work gettin' this party rollin'! 🎉

I see that the Snyk and CodeRabbit checks have passed with flying colors. However, I noticed the DCO (Developer Certificate of Origin) check is showin' "ACTION_REQUIRED". You might want to double-check that before the maintainers can merge this beauty.

The maintainers @mo-auto and @yurem will need to give their human review and approval before this can be merged. You better think twice and make sure everything's in order!

Keep up the good work on cleanin' up those docs! 📚

(\_(\ 
(='.'=) 
(")_(")

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current status of PR #12743 including checks, reviews, and merge status
gh pr view 12743 --json number,title,state,mergeable,reviewDecision,statusCheckRollup,reviews --repo JanssenProject/jans

# Get more detailed information about the status checks
gh pr checks 12743 --repo JanssenProject/jans

Length of output: 11117


@Vijayshree25, listen up, my friend! Like a wise ol' pigeon, I've checked the current status of your PR and here's what I'm seein':

Good news 🎉:

  • ✅ I did approve the PR (latest review on commit 69e9104d)
  • ✅ Snyk security checks passed
  • ✅ The PR is mergeable

Not-so-good news ⚠️:

You better think twice and double-check that DCO! The status shows it hasn't passed yet. You might need to:

  1. Ensure all commits are properly signed off with git commit -s
  2. If you have unsigned commits, you may need to amend them or add a sign-off retroactively

What's needed for merge:

  • The reviewDecision shows "REVIEW_REQUIRED" - maintainer approval from @mo-auto or @yurem is still needed
  • The DCO check needs to pass before they can merge

Once you fix that DCO issue and get maintainer approval, this beauty will be ready to roll! Let's get this party rollin'! 🚀

(\_(\ 
(='.'=) 
(")_(") Ready to help!

@Vijayshree25
Copy link
Author

Hi @mo-auto and @yurem ,

The PR is now ready for merge:

  • All markdown lint issues have been fixed.
  • Commit includes Signed-off-by for DCO.
  • @coderabbitai[bot] has approved the PR.
  • Snyk and CodeRabbit checks have passed.

Could you please review and merge? Thanks!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current comprehensive status of PR #12743
gh pr view 12743 --json number,title,state,mergeable,reviewDecision,statusCheckRollup,reviews,commits --repo JanssenProject/jans

# Get detailed status checks
gh pr checks 12743 --repo JanssenProject/jans

# Check the latest commit to see if it has DCO sign-off
gh api repos/JanssenProject/jans/pulls/12743/commits --jq '.[-1] | {sha: .sha, message: .commit.message, author: .commit.author}'

Length of output: 12534


@Vijayshree25, listen up, my friend! Like a wise ol' pigeon, I've checked the current status and here's what I'm seein':

Good news 🎉:

  • ✅ I confirmed my approval on your latest commit (69e9104)
  • ✅ Snyk checks passed
  • ✅ CodeRabbit checks passed
  • ✅ The PR is technically mergeable
  • ✅ Your latest commit includes the Signed-off-by line

The DCO situation 🤔:
The DCO check is still showing as "ACTION_REQUIRED" / fail status, even though your latest commit has the Signed-off-by. This might be because:

  1. One or more of your earlier commits may not have the sign-off
  2. The DCO check may need to re-evaluate all commits

You have 3 commits in this PR. You better think twice and verify that all three commits have the DCO sign-off, not just the latest one.

What's still needed:

  • ❗ Fix the DCO check (currently failing)
  • ❗ Maintainer approval from @mo-auto or @yurem (reviewDecision shows "REVIEW_REQUIRED")

To fix the DCO:
If earlier commits are missing the sign-off, you may need to:

git rebase HEAD~3 --signoff
git push --force-with-lease

Once that DCO check turns green and you get maintainer approval, this beauty will be ready to roll! Let's get this party rollin'! 🚀

(\_(\ 
(='.'=) 
(")_(") Ready to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants