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

Adding both WebSocket tests to /test-integration/ to verify behavior that is causing problems from within OSGi. #3172

Merged
merged 1 commit into from Mar 21, 2019

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Nov 30, 2018

  • Renamed TestableJettyServer to XmlBasedJettyServer to avoid
    it being picked up as a Test class (due it starting with the
    name "Test")
  • Renamed .addConfiguration() to .addXmlConfiguration() to
    reflect true nature, and also to avoid naming confusion
    with new Configuration infrastructure in Jetty.

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

@joakime
Copy link
Contributor Author

joakime commented Nov 30, 2018

There's no changes to anything outside of /test-integration/ but I was able to narrow down a culprit in the /test-jetty-osgi/ issues we are having in the jetty-10.0.x-3162_jetty_servlet_api branch to following XML snippet.

<Configure id="Server" class="org.eclipse.jetty.server.Server">
    <!-- =========================================================== -->
    <!-- Set up the list of default configuration classes            -->
    <!-- =========================================================== -->    
    <Call class="org.eclipse.jetty.webapp.Configurations" name="setKnown">
      <Arg>
        <Array type="String">
         <Item>org.eclipse.jetty.webapp.FragmentConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.JettyWebXmlConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.WebXmlConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.WebAppConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.ServletsConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.JspConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.JaasConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.JndiConfiguration</Item>
         <Item>org.eclipse.jetty.plus.webapp.PlusConfiguration</Item>
         <Item>org.eclipse.jetty.plus.webapp.EnvConfiguration</Item>
         <Item>org.eclipse.jetty.webapp.JmxConfiguration</Item>
         <Item>org.eclipse.jetty.osgi.annotations.AnnotationConfiguration</Item>
         <Item>org.eclipse.jetty.websocket.server.JettyWebSocketConfiguration</Item>
         <Item>org.eclipse.jetty.websocket.jsr356.server.JavaxWebSocketConfiguration</Item>
         <Item>org.eclipse.jetty.osgi.boot.OSGiWebInfConfiguration</Item>
         <Item>org.eclipse.jetty.osgi.boot.OSGiMetaInfConfiguration</Item>
        </Array>
      </Arg>
    </Call>
</Configure>

When this Configurations.setKnown() is used the other branch fails websocket, seemingly due to mangled SCI execution.

@joakime joakime requested a review from gregw January 8, 2019 21:58
@gregw
Copy link
Contributor

gregw commented Jan 16, 2019

@joakime do you want to freshen this up before I review?

@janbartel
Copy link
Contributor

@joakime is there an issue still with websocket and osgi for jetty-10? I thought it was all fixed.

@joakime
Copy link
Contributor Author

joakime commented Jan 29, 2019

@janbartel this PR isn't directly for OSGi, just adding some /test-integration/ tests for websocket to verify websocket behavior first.
Before we tackled the OSGi specific issues (which you have already resolved).

The tests that this PR introduce into /test-integration/ still has merit IMO.

that is causing problems from within OSGi.

+ Renamed TestableJettyServer to XmlBasedJettyServer to avoid
  it being picked up as a Test class (due it starting with the
  name "Test")
+ Renamed .addConfiguration() to .addXmlConfiguration() to
  reflect true nature, and also to avoid naming confusion
  with new Configuration infrastructure in Jetty.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime force-pushed the jetty-10.0.x-integration-testing-websockets branch from f42fa06 to b351631 Compare March 21, 2019 19:31
@joakime joakime merged commit aa66756 into jetty-10.0.x Mar 21, 2019
@joakime joakime deleted the jetty-10.0.x-integration-testing-websockets branch March 21, 2019 19:32
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.

None yet

3 participants