Skip to content

Conversation

@JeremySun0731
Copy link

I added a focused unit test (ReflectionDiffBuilderAccessibilityTest) to demonstrate that
ReflectionDiffBuilder still forces reflective access to private fields even when
force-access is disabled.

This test shows that ReflectionDiffBuilder has not yet been migrated to the opt-in
accessibility controls introduced via AbstractReflection in PR #1558, and still relies
on legacy forced reflection behavior.

The test is intentionally minimal and serves as executable evidence of the current
behavior discussed in LANG-1711.

@garydgregory
Copy link
Member

@JeremySun0731

This looks like it duplicates most of #1558

Instead you should create a PR against my branch, otherwise I can't tell what you've added or changed.

@JeremySun0731
Copy link
Author

Thanks for the clarification!You're right — the PR was based on master, which makes the diff include most of #1558.
I'll rework this as a PR against your branch so that it only shows the additional test demonstrating the ReflectionDiffBuilder behavior.

@JeremySun0731
Copy link
Author

I’ve split out the test into a separate commit and based it on
LANG-1711-reflection-diff-safe.
After cherry-picking, it turns out the test commit is already
contained in that branch, so GitHub shows no diff when comparing.
Functionally, the test-only change is identical and isolated as requested.
Please let me know if you’d prefer it to be submitted against a different base.

@garydgregory
Copy link
Member

There are still 37 files changed and 16 commits which can't be right. You likely want the PR to be against my branch (in my fork).

@JeremySun0731
Copy link
Author

Thanks for confirming — that makes sense.

I double-checked the history and confirmed that the test-only commit is already
present in your LANG-1711-reflection-diff-safe branch, which explains why GitHub
shows a large diff when comparing against apache:master and no diff when rebased.
Since the requested behavior is already covered upstream, I’m happy to close
this PR unless you’d prefer it to be submitted against a different base.

@garydgregory
Copy link
Member

There are still 37 files changed and 16 commits which can't be right. You likely want the PR to be against my branch (in my fork).

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.

2 participants