Skip to content

CAMEL-23912: Address review feedback on ThemeTest fix#24482

Merged
davsclaus merged 1 commit into
apache:mainfrom
gnodet:fix-themetest-review-feedback
Jul 7, 2026
Merged

CAMEL-23912: Address review feedback on ThemeTest fix#24482
davsclaus merged 1 commit into
apache:mainfrom
gnodet:fix-themetest-review-feedback

Conversation

@gnodet

@gnodet gnodet commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Claude Code on behalf of gnodet

Summary

Follow-up to #24475 — addresses review feedback from Copilot and @ammachado that landed after the original PR was merged.

Changes:

  • Handle null classloader (Copilot): Theme.class.getClassLoader() can legally return null for bootstrap-loaded classes. Now null-checked before calling getResourceAsStream(), falling through to the TCCL fallback instead of throwing an NPE.
  • Soften javadoc wording (@ammachado suggestion Handle Jetty's removal of method ServletContextHandler.addFilter(String,String,int) #1): Changed from "guaranteed to have access" to accurately describe the fallback behavior, so a future reader doesn't strip the TCCL fallback as dead code.
  • Revert ConcurrentHashMap to HashMap (@ammachado suggestion Camel ssh component CAMEL-4713 #2): All CACHE access is already inside synchronized methods, so the concurrent map added no real safety — just a misleading signal.

Test plan

  • All 6 ThemeTest tests pass.
  • mvn formatter:format impsort:sort produces no diff.

🤖 Generated with Claude Code

- Handle null classloader from Theme.class.getClassLoader() to avoid
  NPE when the class is bootstrap-loaded (Copilot feedback)
- Soften javadoc wording: classloader access is not "guaranteed", it
  falls back to TCCL when the primary lookup returns null (ammachado)
- Revert ConcurrentHashMap to HashMap since all CACHE access is already
  inside synchronized methods (ammachado)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet gnodet requested review from ammachado and davsclaus July 7, 2026 05:18
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions github-actions Bot added the dsl label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • dsl/camel-jbang/camel-jbang-plugin-tui

🔬 Scalpel shadow comparison — Scalpel: 2 tested, 1 compile-only — current: 2 all tested

Maveniverse Scalpel detected 3 affected modules (current approach: 2).

⚠️ Modules only in Scalpel (1)
  • camel-launcher

Skip-tests mode would test 2 modules (1 direct + 1 downstream), skip tests for 1 (generated code, meta-modules)

Modules Scalpel would test (2)
  • camel-jbang-plugin-tui
  • camel-launcher-container
Modules with tests skipped (1)
  • camel-launcher

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (3 modules)
  • Camel :: JBang :: Plugin :: TUI
  • Camel :: Launcher
  • Camel :: Launcher :: Container

⚙️ View full build and test results

@davsclaus davsclaus merged commit 145871e into apache:main Jul 7, 2026
5 checks passed
@davsclaus davsclaus added this to the 4.22.0 milestone Jul 7, 2026
@davsclaus davsclaus added the bug Something isn't working label Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dsl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants