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

Issue #5137 - Cleanup of WebAppContextTest #5141

Closed
wants to merge 0 commits into from
Closed

Issue #5137 - Cleanup of WebAppContextTest #5141

wants to merge 0 commits into from

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Aug 12, 2020

Signed-off-by: Joakim Erdfelt joakim.erdfelt@gmail.com

@joakime joakime self-assigned this Aug 12, 2020
@joakime joakime added the Test label Aug 12, 2020
@joakime joakime linked an issue Aug 12, 2020 that may be closed by this pull request
WebAppContext Tests need cleanup #5137
Closed
@joakime joakime mentioned this pull request Aug 12, 2020
Closed
gregw
gregw requested changes Aug 12, 2020
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

mostly good... just some niggles and a suggestion to test a bit more of ContextListeners

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextListenerTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HttpSessionListenerTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java Outdated Show resolved Hide resolved
@joakime joakime requested a review from gregw August 28, 2020 15:32
janbartel
janbartel requested changes Sep 1, 2020
View reviewed changes
...ebapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/AnnotatedListener.java Outdated Show resolved Hide resolved
...est-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/TestListener.java Outdated Show resolved Hide resolved
jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextListenerTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppClassLoaderTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java Outdated Show resolved Hide resolved
...test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionListenerTest.java Outdated Show resolved Hide resolved
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextExtraClasspathTest.java Outdated Show resolved Hide resolved
gregw
gregw requested changes Sep 21, 2020
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

I still believe we should be testing ordering.
I'm going to bump this to 9.4.33 so we can discuss more.

jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppClassLoaderTest.java Outdated Show resolved Hide resolved
@joakime joakime closed this Sep 22, 2020
@joakime joakime force-pushed the jetty-9.4.x-5137-webappcontext-test-cleanup branch from 9803ca8 to a99a832 Compare September 22, 2020 13:49
@joakime joakime deleted the jetty-9.4.x-5137-webappcontext-test-cleanup branch September 22, 2020 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw approved these changes

@janbartel janbartel Awaiting requested review from janbartel

Assignees

@joakime joakime

Labels
Test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WebAppContext Tests need cleanup
3 participants
@joakime @gregw @janbartel