lhotari opened a new pull request, #25636:
URL: https://github.com/apache/pulsar/pull/25636

   ### Motivation
   
   The Pulsar codebase has accumulated many tests that use reflection 
(`WhiteboxImpl.getInternalState`, `setAccessible(true)`, etc.) to read or 
mutate private fields of the class under test. This pattern was discussed on 
the dev@ mailing list:
   
   [\[DISCUSS\] Stop using reflection to access private fields in 
tests](https://lists.apache.org/thread/7gr04sqmzyttx4ln6ydtp3qv0xgo1o6m)
   
   Reflection-based test access:
   
   - breaks during refactoring without any compile-time signal — renaming or 
retyping a field silently invalidates the test
   - produces verbose, brittle, and hard-to-read test code
   - couples tests to implementation details that should be free to change
   
   The discussion concluded that the preferred approach is to expose what tests 
legitimately need through **package-private** methods annotated with 
`@VisibleForTesting`, with the test placed in the same package as the class 
under test.
   
   This PR teaches the Copilot PR-review bot to flag reflection-based test 
access in new code, so the project guidance is enforced consistently in reviews 
going forward.
   
   ### Modifications
   
   `.github/copilot-instructions.md`:
   
   - Added an **Avoid Reflection in Tests** subsection under *Testing 
Guidelines* with the rationale, the preferred `@VisibleForTesting` 
package-private pattern, before/after examples (`WhiteboxImpl.getInternalState` 
vs. a typed getter), and a link to the dev@ thread.
   - Added a matching item to the **Pull Request Review Guidance** checklist so 
reviewers proactively flag new reflection use.
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   This change is a trivial rework / code cleanup without any test coverage. It 
only modifies Copilot reviewer guidance in `.github/copilot-instructions.md`; 
there is no production or test code change.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If the box was checked, please highlight the changes*
   
   - [ ] Dependencies (add or upgrade a dependency)
   - [ ] The public API
   - [ ] The schema
   - [ ] The default values of configurations
   - [ ] The threading model
   - [ ] The binary protocol
   - [ ] The REST endpoints
   - [ ] The admin CLI options
   - [ ] The metrics
   - [ ] Anything that affects deployment


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to