I tried that and of course it didn't work, but because of lines like these
in TestRuleSetupAndRestoreClass:

      codec = Codec.forName("Lucene40");
      assert codec instanceof Lucene40RWCodec : "fix your classpath to have
tests-framework.jar before lucene-core.jar";
      assert (PostingsFormat.forName("Lucene40") instanceof
Lucene40RWPostingsFormat) : "fix your classpath to have tests-framework.jar
before lucene-core.jar";

I understand the third line, but why do we need to do Codec.forName here?
I searched for Codec.forName and there are 14 matches, some of which are in
tests which should just do e.g. "new SimpleTextCodec()" or "new
Lucene40RWCodec()". What do you think?

Shai


On Sat, Aug 31, 2013 at 6:13 AM, Shai Erera <[email protected]> wrote:

> On LUCENE-5189 I ran into the following problem: I wrote a test which
> creates an index with "old" unsupported Codecs (40, 41, 42) and then tries
> to update them, expecting to fail since those Codecs don't support
> numeric-dv-updates, as well as their Consumer isn't even shipped and they
> throw UOE in fieldsConsumer(). But, the test doesn't hit that exception,
> because the RWCodecs are listed under test-framework/META-INF/services and
> are used instead of the real 40/41/42Codec.
>
> In light of what was discussed here, why do we need those Codecs listed in
> services?
>
> Shai
>
>
> On Fri, Aug 30, 2013 at 5:28 PM, Shai Erera <[email protected]> wrote:
>
>> I removed it from META-INF and all tests pass. So I guess it's safe to
>> remove it completely.
>>
>>
>> Actually Facet45Codec is not needed at all. I dont understand why we
>>> have a codec class here at all.
>>>
>>
>> Because it takes FacetIndexingParams and returns FacetDVF for all fields
>> that appear in the indexing params. Can you do that without a Codec class?
>> It's also easier to use than users figuring they should extend
>> Lucene45Codec to plug-in FacetDVF.
>>
>> I'll just remove it from META-INF?
>>
>> Shai
>>
>>
>> On Fri, Aug 30, 2013 at 5:16 PM, Robert Muir <[email protected]> wrote:
>>
>>> On Fri, Aug 30, 2013 at 10:01 AM, Shai Erera <[email protected]> wrote:
>>> > I don't think we should go that far. If you extend Lucene45Codec you
>>> > basically agree to the entire index format, but are given a chance to
>>> > control per-field postings and doc values. Otherwise, make your own
>>> Codec,
>>> > and then you'll need to register it in META-INF/services.
>>> >
>>> > The assert I proposed to make in the ctor is only for "education
>>> purposes"
>>> > -- apps need not register their Lucene45CodecExtension in services. We
>>> can
>>> > document it, and assertions would help verify it.
>>> >
>>>
>>> I don't think we need anything here really: Facet45Codec shouldnt be
>>> in META-INF.
>>>
>>> Actually Facet45Codec is not needed at all. I dont understand why we
>>> have a codec class here at all.
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [email protected]
>>> For additional commands, e-mail: [email protected]
>>>
>>>
>>
>

Reply via email to