apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.63k stars 1.02k forks source link

Use MockAnalyzers in lucene/facet tests [LUCENE-4701] #5766

Closed asfimport closed 11 years ago

asfimport commented 11 years ago

It'd be nicer to use MockAnalyzer in facet tests instead of specific analyzers from analyzers-common module.


Migrated from LUCENE-4701 by Tommaso Teofili (@tteofili), resolved Jan 21 2013 Attachments: LUCENE-4701.patch (versions: 2)

asfimport commented 11 years ago

Tommaso Teofili (@tteofili) (migrated from JIRA)

and here's the related patch.

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

Great Tommaso. I was going to handle that under #5071, so could you just drop a comment there about the analyzers dependency removal, after you commit this change?

asfimport commented 11 years ago

Robert Muir (@rmuir) (migrated from JIRA)

does this really work... What about the analyzer usage in facet/examples (which should move to demo/ imo) ?

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

... facet/examples (which should move to demo/ imo)

that's the part that's still needed to be done, under #5071. I haven't forgotten about it :).

asfimport commented 11 years ago

Tommaso Teofili (@tteofili) (migrated from JIRA)

at the moment the analyzers-common dependency cannot be completely removed since it's needed by DirectoryTaxonomyWriter which internally uses a KeywordAnalyzer in its IndexWriterConfig so we can get rid of that only if that's made configurable (so we pass an Analyzer in the DTW constructor).

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

I ran facet tests (which cover examples too) and they pass. Seems like not all analyzer dependencies were removed from build.xml. Working on that now.

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

needed by DirectoryTaxonomyWriter which internally uses a KeywordAnalyzer

hmmm, right. I set the Analyzer used by DirTaxoWriter to null, and tests pass. However, as long as examples are still under facet/, we need that dependency in build.xml. It will be removed as part of #5071.

I will post a patch w/ my update shortly.

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

Removes KeywordAnalyzer from DirTaxoWriter.

Also, I restored the dependency on analyzers in build.xml under examples.classpath, since they do depend on it. Once #5071 is done, examples will be removed entirely under demo/ and this dependency will go away w/ them.

A general statement about this dependency, I don't think it's a bad one, not for tests anyway. And maybe DirTaxoWriter will need it in the future too. I'm fine w/ removing it for now, but note that it may be restored in the future, if a real need arises.

asfimport commented 11 years ago

Tommaso Teofili (@tteofili) (migrated from JIRA)

I'm not against having analyzers-common as a dependency if that's needed (even in the main facet module) so I'm fine if that needs to be re-added in the future. Shai, can you think of any problems with regard to the null Analnyzer initialization in the DTW IndexWriterConfig?

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

Shai, can you think of any problems with regard to the null Analnyzer initialization in the DTW IndexWriterConfig?

Now not, I think because of the changes to FieldType in Lucene 4.0. I.e., DirTaxoWriter adds indexed but not tokenized fields. Previously, you had to set an Analyzer, even if you added NOT_ANALYZED_NO_NORMS, or otherwise you'd hit NPE.

If the tests pass, it means it's ok, because DTW is not extendable in a way that it allows you to control how documents are added. But if in the future we'll need to, we'll add this dependency back.

asfimport commented 11 years ago

Tommaso Teofili (@tteofili) (migrated from JIRA)

ok cool, so I think we can go on committing your patch.

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

yes go ahead. when we separate examples (requires changing some big tests that should stay under /facet), we'll know if the dissection is successful :).

asfimport commented 11 years ago

Commit Tag Bot (migrated from JIRA)

[trunk commit] Tommaso Teofili http://svn.apache.org/viewvc?view=revision&revision=1436346

[LUCENE-4701] - applied Shai's patch for using MockAnalyzer in tests and keeping analyzers-common dep only in examples classpath

asfimport commented 11 years ago

Shai Erera (@shaie) (migrated from JIRA)

Are you going to fix it for trunk too?

asfimport commented 11 years ago

Tommaso Teofili (@tteofili) (migrated from JIRA)

yep, I just committed for both trunk and branch_4x.

asfimport commented 11 years ago

Commit Tag Bot (migrated from JIRA)

[branch_4x commit] Tommaso Teofili http://svn.apache.org/viewvc?view=revision&revision=1436355

[LUCENE-4701] - merged back to branch_4x

asfimport commented 11 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

Closed after release.