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.

Sign up for GitHub

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

Jump to bottom

WebAppContext Tests need cleanup #5137

Closed
joakime opened this issue Aug 11, 2020 · 2 comments
Closed

WebAppContext Tests need cleanup #5137

joakime opened this issue Aug 11, 2020 · 2 comments
Assignees
@joakime
Labels
Test

Comments

@joakime
Copy link
Contributor

joakime commented Aug 11, 2020

As pointed out by @janbartel in PR #5131, the WebAppContext tests are a bit messy right now.

This is a tracking issue to cleanup the WebAppContext tests to be more sane and easier to maintain.
@joakime joakime mentioned this issue Aug 11, 2020
Merged
@joakime joakime self-assigned this Aug 12, 2020
joakime added a commit that referenced this issue Aug 12, 2020
@joakime
Issue #5137 - Cleanup of WebAppContextTest
fbefe0b 
Signed-off-by: Joakim Erdfelt <[email protected]>
@joakime joakime linked a pull request Aug 12, 2020 that will close this issue
Issue #5137 - Cleanup of WebAppContextTest #5141
Closed
@joakime
Copy link
Contributor Author

joakime commented Aug 12, 2020

Opened PR #5141

@joakime joakime added the Test label Aug 12, 2020
joakime added a commit that referenced this issue Aug 12, 2020
@joakime
Issue #5137 - Moving HttpSessionListenerTest contents to existing Ses…
046726c 
…sionListenerTest

+ Cleanup of SessionListenerTest with eye to resource cleanup

Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Sep 16, 2020
@joakime
Issue #5137 - Removing ServletContextListenerTest as it duplicates tests
ad7ab4e 
Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Sep 16, 2020
@joakime
Issue #5137 - Restoring WebAppClassLoaderTest test method
8166f7f 
+ `ordering()` is now `testClashingResource()` and properly
  tests assumptions when dealing with a ClassLoader.
  The old test assumed guaranteed order from a ClassLoader,
  which is not a feature of any Java ClassLoader.
  Especially so for URLClassLoader.

Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Sep 16, 2020
@joakime
Issue #5137 - Adding test case to demonstrate usage.
61ffdc2 
+ Partially reverting change in commit 31175e9

Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Sep 21, 2020
@joakime
Issue #5137 - Restoring clashing resource test
f5a8995 
Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Sep 22, 2020
@joakime
Issue #5137 - Cleanup of WebAppContextTest
a99a832 
+ Cleanup of test-servlet-spec webapp
+ Removing unused methods
+ Commented System.err.println moved to java.util.logging
+ General checkstyle cleanup (empty lines, spacing, etc)
+ Eliminate bogus WebApp LIB ordering tests.
+ No ClassLoader has a order guarantee.
+ Moving HttpSessionListenerTest contents to existing SessionListenerTest
+ Cleanup of SessionListenerTest with eye to resource cleanup
+ Removing ServletContextListenerTest as it duplicates tests
+ Restoring WebAppClassLoaderTest test method
+ `ordering()` is now `testClashingResource()` and properly
  tests assumptions when dealing with a ClassLoader.
  The old test assumed guaranteed order from a ClassLoader,
  which is not a feature of any Java ClassLoader.
  Especially so for URLClassLoader.
+ Restoring clashing resource test

Signed-off-by: Joakim Erdfelt <[email protected]>
@joakime
Copy link
Contributor Author

joakime commented Oct 14, 2020

PR #5141 merged.

@joakime joakime closed this as completed Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@joakime joakime

Labels
Test
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue #5137 - Cleanup of WebAppContextTest jetty/jetty.project
1 participant
@joakime