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

JSP TCK Refactoring using Arquillian and JUnit #1182

Merged
merged 17 commits into from
Nov 28, 2023

Conversation

alwin-joseph
Copy link
Contributor

@alwin-joseph alwin-joseph commented Jun 12, 2023

Related Issue(s)
#1126

Describe the change

Pending items :

  • 18 security tests under jsp/src/main/java/com/sun/ts/tests/jsp/spec/security which is run only for full & web platform mode is yet to be refactored. These tests will be tagged "security" so it can be excluded for standalone mode.
  • 10 tests under jsp/src/main/java/com/sun/ts/tests/jsp/spec/tagext/resource which is run only for full platform mode is yet to be refactored. These tests will be tagged "resource" so it can be excluded for standalone and web profile.
  • 1 signature test is pending

: copied the web.xml from src, copied other web resources(tld, tag, tagx, gf, jsp, jspx, html files) from webartifacts/jsp module
- pending 37 tests after this change to be run
- URLClient classes renamed to URLClientIT
@alwin-joseph alwin-joseph requested a review from olamy June 28, 2023 13:23
@alwin-joseph alwin-joseph marked this pull request as ready for review June 28, 2023 13:24
@scottmarlow
Copy link
Contributor

@markt-asf @stuartwdouglas like other TCK rewriting, the JSP TCK rewrite is going to be too big to review. IMO, when we have time to try running different implementations against rewritten TCKs, we will better be able to review the new TCKs. Having said that, your feedback is welcome on this pr.

Copy link
Contributor

@gurunrao gurunrao left a comment

Choose a reason for hiding this comment

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

code changes looks good to me, I have added minor comments for future maintainability.

@arjantijms
Copy link
Contributor

Maybe these things can (should?) move to the base class of the IT tests?

@BeforeEach
   void logStartTest(TestInfo testInfo) {
     logger.log(Logger.Level.INFO, "STARTING TEST : "+testInfo.getDisplayName());
   }

   @AfterEach
   void logFinishTest(TestInfo testInfo) {
     logger.log(Logger.Level.INFO, "FINISHED TEST : "+testInfo.getDisplayName());
   }

@alwin-joseph
Copy link
Contributor Author

@arjantijms @olamy I have added the changes to use host/port from Arquillian only in latest commit. If no other comments I will merge this PR by tomorrow.

@dmatej I re-requested review by mistake. Please ignore.

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.

7 participants