Solr
  1. Solr
  2. SOLR-5771

Add SuppressSSL instead of static boolean in SolrTestCaseJ4

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.8, 6.0
    • Component/s: Tests
    • Labels:
      None

      Description

      Currently, as of 4.7, solr test-framework classes can no longer be used by downstream projects, because ssl configuration will fail (and it expects this stuff from outside the source code tree in example/ etc, which makes it impossible).

      There is a boolean to disable SSL, but it cannot work correctly unless you set it in a static initializer (to "beat" the SolrTestCaseJ4.beforeClass to the punch). Then the problem is afterClass turns it off, so if you have e.g. a base class run by 2 tests in the same jvm, boom.

      An alternative way, so people can use test-framework again, is to just have an annotation to do this. Thats how e.g. codecs and so on are disabled in lucene.

        Issue Links

          Activity

          Hide
          Robert Muir added a comment -

          heres a patch to start with... seems to work...

          Show
          Robert Muir added a comment - heres a patch to start with... seems to work...
          Hide
          Uwe Schindler added a comment -

          Thanks for bringing this up! This static pattern looked so wrong to me! I was confronted by this on the weekend when I cleaned up the morphlines code, but I was afraid to touch it; wanted to come back to it this week. Your fix is so wonderful and looks like syntactic sugar!

          Uwe

          ^^^ is this enough for @UweSays?

          Show
          Uwe Schindler added a comment - Thanks for bringing this up! This static pattern looked so wrong to me! I was confronted by this on the weekend when I cleaned up the morphlines code, but I was afraid to touch it; wanted to come back to it this week. Your fix is so wonderful and looks like syntactic sugar! Uwe ^^^ is this enough for @UweSays?
          Hide
          Mark Miller added a comment -

          Thanks!

          Don't really know the annotations life cycle, so the static was all I knew of to get ahead of beforeClass. It was ugly (the other guy working on SSL recoiled as well, but you had to disable before beforeClass). As Robert pointed out though, there is a problem with this and inheritance.

          An annotation that can jump in before beforeClass is much nicer and actually works properly.

          Show
          Mark Miller added a comment - Thanks! Don't really know the annotations life cycle, so the static was all I knew of to get ahead of beforeClass. It was ugly (the other guy working on SSL recoiled as well, but you had to disable before beforeClass). As Robert pointed out though, there is a problem with this and inheritance. An annotation that can jump in before beforeClass is much nicer and actually works properly.
          Hide
          Gregory Chanan added a comment -

          We ran into this as well with Apache Sentry, thanks for fixing it!

          Is it expected that downstream projects should add in the annotation? As you noted, ssl configuration will fail because it expects stuff from example/ – we can't test for this and automatically disable?

          Show
          Gregory Chanan added a comment - We ran into this as well with Apache Sentry, thanks for fixing it! Is it expected that downstream projects should add in the annotation? As you noted, ssl configuration will fail because it expects stuff from example/ – we can't test for this and automatically disable?
          Hide
          Robert Muir added a comment -

          Is it expected that downstream projects should add in the annotation? As you noted, ssl configuration will fail because it expects stuff from example/ – we can't test for this and automatically disable?

          No. This issue is just to replace the static boolean with an annotation. For me, thats enough, as it makes the solr code itself cleaner, and it also gives downstream projects back the same capabilities they had in 4.6 and before.

          Yes its true, in practice you will have to do this right now if you want to use these test classes all in an outside project. As far as autodetection, I don't think tests should automatically 'pass' when misconfigured: that makes them useless. I also think things like example keystores are useless too, and that the only proper fix is to move that "test keystore" to a src/resources in the test-framework.jar. But personally I don't want to tackle all this (please open an issue if you do!)

          Show
          Robert Muir added a comment - Is it expected that downstream projects should add in the annotation? As you noted, ssl configuration will fail because it expects stuff from example/ – we can't test for this and automatically disable? No. This issue is just to replace the static boolean with an annotation. For me, thats enough, as it makes the solr code itself cleaner, and it also gives downstream projects back the same capabilities they had in 4.6 and before. Yes its true, in practice you will have to do this right now if you want to use these test classes all in an outside project. As far as autodetection, I don't think tests should automatically 'pass' when misconfigured: that makes them useless. I also think things like example keystores are useless too, and that the only proper fix is to move that "test keystore" to a src/resources in the test-framework.jar. But personally I don't want to tackle all this (please open an issue if you do!)
          Hide
          ASF subversion and git services added a comment -

          Commit 1571691 from Robert Muir in branch 'dev/trunk'
          [ https://svn.apache.org/r1571691 ]

          SOLR-5771: Add SuppressSSL instead of static boolean in SolrTestCaseJ4

          Show
          ASF subversion and git services added a comment - Commit 1571691 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1571691 ] SOLR-5771 : Add SuppressSSL instead of static boolean in SolrTestCaseJ4
          Hide
          ASF subversion and git services added a comment -

          Commit 1571695 from Robert Muir in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1571695 ]

          SOLR-5771: Add SuppressSSL instead of static boolean in SolrTestCaseJ4

          Show
          ASF subversion and git services added a comment - Commit 1571695 from Robert Muir in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1571695 ] SOLR-5771 : Add SuppressSSL instead of static boolean in SolrTestCaseJ4
          Hide
          Uwe Schindler added a comment -

          Close issue after release of 4.8.0

          Show
          Uwe Schindler added a comment - Close issue after release of 4.8.0

            People

            • Assignee:
              Unassigned
              Reporter:
              Robert Muir
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development