Skip to content

Merge jakarta.web.beans and web.beans#7958

Merged
matthiasblaesing merged 4 commits intoapache:masterfrom
matthiasblaesing:merge_cdi2
Dec 22, 2024
Merged

Merge jakarta.web.beans and web.beans#7958
matthiasblaesing merged 4 commits intoapache:masterfrom
matthiasblaesing:merge_cdi2

Conversation

@matthiasblaesing
Copy link
Contributor

@matthiasblaesing matthiasblaesing commented Nov 12, 2024 •
edited
Loading

The jakarta.web.beans module was created as a copy of the web.beans
module, just using the new namespaces after the death of JavaEE and the
inception of JakartaEE. Other modules followed a different route, where
the existing implementations were extended to support both the old and
new XML/package namespaces.

This becomes a problem, when it hits the UI as the user then does not
know what to do (which template/menu entry to choose and "what the heck
is JakartaEE and why is that not JavaEE anymore?!"). What is more
codepaths are duplicated and fixes for one area are highly likely to be
relevant for the other also.

This change integrates the unittests from the jakarta.web.beans project,
if they actually check jakarta* functions and modifies the codepaths, so
that both namespaces are supported.

This PR also fixes and reenables the CI/CD integration of the web.beans tests.

Closes: #7918
Closes: #6875

@matthiasblaesing matthiasblaesing added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Java EE/Jakarta EE [ci] enable enterprise job enterprise [ci] enable enterprise job labels Nov 12, 2024
@matthiasblaesing matthiasblaesing added this to the NB25 milestone Nov 12, 2024
@matthiasblaesing matthiasblaesing added the ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) label Nov 13, 2024
@apache apache locked and limited conversation to collaborators Nov 13, 2024
@apache apache unlocked this conversation Nov 13, 2024
This was referenced Nov 13, 2024
Closed
Closed
mbien
mbien reviewed Nov 22, 2024
View reviewed changes
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

i think this makes sense since the copied modules would likely never diverge enough to warrant having two impls. Only looked through this on github so far.

@matthiasblaesing
Copy link
Contributor Author

matthiasblaesing commented Nov 23, 2024

@mbien thanks for the suggestion. Pushed an update, might squashed with b4c6e55 before merge.

@mbien mbien added tests JavaDoc [ci] enable java/javadoc tests and build-javadoc target labels Dec 2, 2024
mbien
mbien approved these changes Dec 2, 2024
View reviewed changes
Copy link
Member

@mbien mbien left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looked through everything with a coffee and I think it should be fine.

left some comments but they aren't that important. List.of() is immutable and has the advantage that it makes followup List.copyOf()'s a no-op. It also doesn't allow null, so it would fail early.

@matthiasblaesing
Copy link
Contributor Author

matthiasblaesing commented Dec 3, 2024

@mbien thank you. I updated the PR and plan to squash 24e6f9b into b4c6e55 before merge.

@matthiasblaesing
Fix CDI Unittests
2b82c08 
Start: 85 passed, 45 failed

- Add "Java SE Platforms and Libraries" as test dependency

Status: 125 passed, 5 failed

- Reference code uses wrong placement of annotations, method annotations
  come before the return type

- There is not mention in the spec, that an @Injectable field may not be
  initialized

Status: 130 passed
@matthiasblaesing
Merge jakarta.web.beans and web.beans
fcdd3e8 
The jakarta.web.beans module was created as a copy of the web.beans
module, just using the new namespaces after the death of JavaEE and the
inception of JakartaEE. Other modules followed a different route, where
the existing implementations were extended to support both the old and
new XML/package namespaces.

This becomes a problem, when it hits the UI as the user then does not
know what to do (which template/menu entry to choose and "what the heck
is JakartaEE and why is that not JavaEE anymore?!"). What is more
codepaths are duplicated and fixes for one area are highly likely to be
relevant for the other also.

This change integrates the unittests from the jakarta.web.beans project,
if they actually check jakarta* functions and modifies the codepaths, so
that both namespaces are supported.

Closes: apache#7918
@matthiasblaesing
Reenable web.beans tests
1e3bbde 
Local run is clean, runtime is high, but seems reasonable (5:30min)
@matthiasblaesing
web.beans: Simplify/consolidate annotation analysis
d31d18a
@matthiasblaesing matthiasblaesing added the ci:all-tests [ci] enable all tests label Dec 21, 2024
@apache apache locked and limited conversation to collaborators Dec 21, 2024
@apache apache unlocked this conversation Dec 21, 2024
@matthiasblaesing matthiasblaesing added the LSP [ci] enable Language Server Protocol tests label Dec 21, 2024
@matthiasblaesing
Copy link
Contributor Author

matthiasblaesing commented Dec 21, 2024

Squashed and rebased onto master, resolving the merge conflict. I removed the reference to jakarta.web.beans from the lsp build and requested a full test run with this.

I intent to merge this shortly when/if tests are green.

@mbien
Copy link
Member

mbien commented Dec 22, 2024 •
edited
Loading

the failing test seems to be a real issue, but unrelated to this PR. Have to check when this started. sorry for the noise, I was just unlucky during restart. It failed in two PRs at the same time so I got paranoid.

@matthiasblaesing
Copy link
Contributor Author

matthiasblaesing commented Dec 22, 2024

@mbien thanks for having a look. No need to apologize, flaky tests are ugly. I ran the tests locally and apart from locale problems (since a few releases javac localizes messages) and the excessive runtime, everything seems to be ok. So I'll merge.

@matthiasblaesing matthiasblaesing merged commit d26b090 into apache:master Dec 22, 2024
41 of 68 checks passed
@mbien mbien mentioned this pull request Dec 26, 2024
Merged
@matthiasblaesing matthiasblaesing deleted the merge_cdi2 branch December 27, 2024 21:11
@mbien mbien mentioned this pull request May 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbien mbien mbien approved these changes

@juneau001 juneau001 Awaiting requested review from juneau001

@pepness pepness Awaiting requested review from pepness

Assignees

No one assigned

Labels

ci:all-tests [ci] enable all tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) enterprise [ci] enable enterprise job Java EE/Jakarta EE [ci] enable enterprise job Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) JavaDoc [ci] enable java/javadoc tests and build-javadoc target LSP [ci] enable Language Server Protocol tests tests

Projects

NetBeans JavaEE related
Status: Done

Milestone

NB25

Development

Successfully merging this pull request may close these issues.

Navigate context menu contains duplicated entries 2 "Contexts and Dependency Injection" choices for New

2 participants

@matthiasblaesing @mbien

Comments