Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WELD-2771 Revisit Weld examples, update deps, verify they work #2958

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

manovotn
Copy link
Contributor

@manovotn manovotn commented Apr 17, 2024

Examples are very outdated and don't even work with latest JDKs.
We should pick some minimal set of examples that demonstrate - EE usage, servlet usage, SE usage (, maybe the Groovy SE example?).

This can be done by keeping the Numberguess example for both, SE and JSF variants.

Things to do:

  • Remove JSF login and translator examples
    • Remove mentions of the above in our docs
    • Add a link to WFLY quickstarts as a source of more examples; many of them use CDI anyway
  • Update JSF Numberguess to work with WFLY, Tomcat and Jetty, update its README
  • Update test setup of JSF Numberguess to strip away Arq.
    • Copied over from the same example under WFLY - kept some basic assertions against running deployed application
  • Update and test SE Numberguess sample
  • Update and test SE Groovy example
  • Scan Weld docs for any other mention of example usage to make sure we don't have leftovers
  • Think if and how we can test this in CI
    • We can surely test a build but deployments might be tricky - each setup requires starting and stopping a container

@manovotn
Copy link
Contributor Author

The failures are related to update of some dependencies which have min. JDK version set to 17
We could extract servlet testing from Weld CI / Weld In-container Tests - JDK 11 into a separate job or simply skip that part if JDK is 11.
As for Weld CI / CDI TCK - JDK 11, this is because we are currently relying on the pre-split version of TCK which has web tests as well and is therefore affected as well. I need to re-check the status of tests moving into umbrella TCK and if there is a release we can consume already.

…d into platform TCKs.

Update CI to skip servlet tests for JDK 11 (some deps require 17 as min version).
@manovotn
Copy link
Contributor Author

I've adjusted the CI in the following way:

  • Examples are now only built (not tested yet) and this is done with JDK 17 only
    • We should still look into how and if we can test it too, which is TBD
  • CDI TCKs are run with all TCKs and should pass since the web profile is now not present at all
    • I didn't find any release of the platform TCK yet but we need to reintroduce that ASAP
    • I should at least try some local setup with snapshots and store it under a JIRA to keep track
  • In-container tests on JDK 11 now skip servlet modules which would require JDK 17+
    • JDK 17 and above tests are not affected in any way

@manovotn
Copy link
Contributor Author

The last thing needed in this PR is testing the examples (running their tests) - I'll give it a spin next week to see if we can get at least some simple setup running.

@manovotn manovotn marked this pull request as ready for review April 22, 2024 12:46
@manovotn
Copy link
Contributor Author

I've added CI setup that performs a Maven build on examples and then starts WFLY as background process, deploys JSF numberguess and runs tests against it. This should give us at least basic level of confidence.
We could add the same with Tomcat/Jetty but the version keeping would be more difficult and job longer due to re-download of servlets on every run. With WFLY it's easier as we already have the patched version prepared from initial job.

@manovotn manovotn merged commit aab0ff0 into weld:master Apr 22, 2024
16 checks passed
@manovotn manovotn deleted the weld2771 branch April 22, 2024 14:10
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.

1 participant