Skip to content

fix and enable unit tests in the web.jsf.editor module - #9617

Open
NicolaIsotta wants to merge 2 commits into
apache:masterfrom
NicolaIsotta:web.jsf.editor-tests
Open

NicolaIsotta wants to merge 2 commits into
apache:masterfrom
NicolaIsotta:web.jsf.editor-tests

Conversation

@NicolaIsotta

Copy link
Copy Markdown
Contributor

This also corrects some regressions discovered with the tests:

  • fix JsfTagFeatureProvider not to work with only the newest namespaces
  • CompositeComponentLibrary.getDefaultNamespace should use LibraryUtils, as before 14fee17
  • ConfigManager's SCHEMA_CACHE must also have the resource name in the key

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

LLMs, Commit messages and PR description:

  • Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.
  • LLM assisted commits should be attributed with an Assisted-by: MODEL_NAME MODEL_VERSION line appended to the commit message.
    • Please mention coding assistance in the PR description too (eg. by adding the same Assisted-by line from above)
    • Please describe the changes in your own words - we'd like to know you understand the changes being made!

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

This also corrects some regressions discovered with the tests:
- fix JsfTagFeatureProvider not to work with only the newest namespaces
- CompositeComponentLibrary.getDefaultNamespace should use LibraryUtils, as before 14fee17
- ConfigManager's SCHEMA_CACHE must also have the resource name in the key
@mbien

mbien commented Sep 15, 2026

Copy link
Copy Markdown
Member

they appear to be disabled in CI atm:

# Fails
# - name: web.jsf.editor
# run: ant $OPTS -f enterprise/web.jsf.editor test

assuming they work now reliably again (please run them a few times in a loop to check), feel free to comment those lines in so that CI can run them.

thanks for working on those updates!

@NicolaIsotta NicolaIsotta changed the title fix unit tests in the web.jsf.editor module fix and enable unit tests in the web.jsf.editor module Sep 16, 2026
@NicolaIsotta

Copy link
Copy Markdown
Contributor Author

I can confirm unit tests work. Tested on different machines.
Integration tests fail, though. I haven't touched them. Does CI runs IT as well? Because in that case they have to be disabled.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

I can confirm unit tests work. Tested on different machines. Integration tests fail, though. I haven't touched them. Does CI runs IT as well? Because in that case they have to be disabled.

It depends 😉. CI does what you tell it to do.

Have a look at the test invocation in main.yml currently there the test target is specified this in turn depends on the two targets test-unit and test-qa-functional. To run only the unittests change the target in main.yml to test-unit.

@mbien

mbien commented Sep 16, 2026

Copy link
Copy Markdown
Member

Does CI runs IT as well? Because in that case they have to be disabled.

well, lets take a look if something fails - approved the workflow run :)

will run it later via #9598 this will tell for sure if CI is ok with that module.

@NicolaIsotta

Copy link
Copy Markdown
Contributor Author

It ran ITs and failed - as expected. Let me know if I can change the target in the workflow file.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

@NicolaIsotta yes please update the workflow and switch it to only run unittests. We gain more from running working unittests, than ignoring all tests of the module.

Make unit-tests work again with code and configuration changes (ITs still fail)

This also corrects some regressions discovered with the tests:
- fix JsfTagFeatureProvider not to work with only the newest namespaces
- CompositeComponentLibrary.getDefaultNamespace should use LibraryUtils, as before 14fee17
- ConfigManager's SCHEMA_CACHE must also have the resource name in the key

Minor cleanup to some test classes
@NicolaIsotta

Copy link
Copy Markdown
Contributor Author

I do agree - enabling unit tests let me found a couple of regressions.
I updated the workflow file

@mbien mbien added this to the NB32 milestone Sep 16, 2026
@mbien

mbien commented Sep 16, 2026

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enterprise [ci] enable enterprise job Java EE/Jakarta EE [ci] enable enterprise job tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants