DYN-6094 Lucene unit testing by reddyashish · Pull Request #14214 · DynamoDS/Dynamo

reddyashish · 2023-08-01T16:27:35Z

Purpose

This PR is to use Lucene search in the Dynamo unit tests.

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated
This PR contains no files larger than 50 MB

Release Notes

DYN-6094 Lucene unit testing

Reviewers

@QilongTang @RobertGlobant20

FYIs

@DynamoDS/dynamo

RobertGlobant20

LGTM with some comments

src/DynamoCore/Models/DynamoModel.cs

RobertGlobant20 · 2023-08-01T17:11:09Z

src/DynamoCore/Models/DynamoModel.cs

-            //}
-
-            var ele = AddNodeTypeToSearch(cbnData);
+            var ele = AddNodeTypeToSearch(outputData);      


just consider (for future bug fixes) that uncommenting this will probably generate duplicates in output and input nodes when searching in the Library (due that are already added in library).

Thanks @RobertGlobant20. Will test this out.

src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs

test/DynamoCoreWpfTests/WorkspaceSaving.cs

QilongTang

LGTM
The regressions reported is because this branch is not up to date with master latest.
just curious did we have to update the result of any previous search tests?

RobertGlobant20

LGTM with one comment

src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs

reddyashish · 2023-08-09T14:19:12Z

@QilongTang updated to latest master now. did not have to change any search results for now.

QilongTang · 2023-08-09T18:50:01Z

hi @reddyashish Let us know when this is ready for one more review

reddyashish · 2023-08-09T22:35:50Z

src/DynamoCore/Utilities/LuceneSearchUtility.cs

+
+
+            // Create an analyzer to process the text
+            Analyzer = CreateAnalyzerByLanguage(dynamoModel.PreferenceSettings.Locale);


Added back the lucene search support for different languages.

@reddyashish you miss to comment/remove the line 86 otherwise we will be using always the StandardAnalyzer.

Addressing it in here: #14248

reddyashish · 2023-08-09T22:36:27Z

src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs

-            if (useLucene)
-            {
-                //The DirectoryReader and IndexSearcher have to be assigned after commiting indexing changes and before executing the Searcher.Search() method, otherwise new indexed info won't be reflected
-                LuceneSearchUtility.dirReader = LuceneSearchUtility.writer?.GetReader(applyAllDeletes: true);


All the search code is moved to the NodeSearchModel class.

reddyashish · 2023-08-09T22:39:45Z

@QilongTang This is now ready for final review
Main changes in recent commit:

Updated SearchModelTests to use the lucene search model.
Moved the lucene search code from ViewModel to Model.
Added back the lucene search support for different languages.

reddyashish · 2023-08-10T01:03:42Z

The failing tests here are from documentationbrowser and not related to this PR.

EngOps build has failed with this error, which is weird as those DLL's are present.

QilongTang · 2023-08-10T13:30:38Z

The failing tests here are from documentationbrowser and not related to this PR.

EngOps build has failed with this error, which is weird as those DLL's are present.

Those binaries are not under AnyCPU fiolder for now

QilongTang · 2023-08-10T15:55:10Z

@reddyashish Did you run self serve on this branch or confirm the testing state? If yes, good to merge

reddyashish · 2023-08-10T17:08:55Z

Yes, I ran the self-serve and only 5 docbrowser tests are failing.

reddyashish · 2023-08-10T17:23:05Z

Merging this and will cherrypick into 2.19

* Lucene unit testing * test updates * Update LuceneSearchUtility.cs * Update NodeAutoCompleteSearchViewModel.cs * Update NodeAutoCompleteSearchViewModel.cs * Test updates (cherry picked from commit 780f854)

* DYN-6094 Lucene unit testing (#14214) * Lucene unit testing * test updates * Update LuceneSearchUtility.cs * Update NodeAutoCompleteSearchViewModel.cs * Update NodeAutoCompleteSearchViewModel.cs * Test updates (cherry picked from commit 780f854) * Update SearchResultDataProvider.cs (#14253) Co-authored-by: pinzart <[email protected]> (cherry picked from commit aaf6a4e) --------- Co-authored-by: pinzart90 <[email protected]>

Lucene unit testing

206add1

RobertGlobant20 approved these changes Aug 1, 2023

View reviewed changes

avidit changed the title ~~Lucene unit testing~~ DYN-6094 Lucene unit testing Aug 2, 2023

test updates

a1e5048

reddyashish commented Aug 2, 2023

View reviewed changes

test/DynamoCoreWpfTests/WorkspaceSaving.cs Show resolved Hide resolved

Update LuceneSearchUtility.cs

8bff217

QilongTang reviewed Aug 8, 2023

View reviewed changes

RobertGlobant20 approved these changes Aug 8, 2023

View reviewed changes

src/DynamoCoreWpf/ViewModels/Search/NodeAutoCompleteSearchViewModel.cs Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into LuceneUnitTesting

9df2c78

reddyashish added 2 commits August 9, 2023 20:17

Update NodeAutoCompleteSearchViewModel.cs

7b5c71e

Update NodeAutoCompleteSearchViewModel.cs

cc44f5f

Test updates

5c261cc

reddyashish commented Aug 9, 2023

View reviewed changes

reddyashish mentioned this pull request Aug 10, 2023

Introducing a global Lucene search singleton class #14248

Merged

9 tasks

QilongTang added this to the 2.19.0 milestone Aug 10, 2023

QilongTang approved these changes Aug 10, 2023

View reviewed changes

reddyashish merged commit 780f854 into DynamoDS:master Aug 10, 2023

reddyashish mentioned this pull request Aug 14, 2023

Cherrypicking lucene unit testing #14257

Merged

9 tasks



		// Create an analyzer to process the text
		Analyzer = CreateAnalyzerByLanguage(dynamoModel.PreferenceSettings.Locale);

Conversation

reddyashish commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

Release Notes

Reviewers

FYIs

Uh oh!

RobertGlobant20 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RobertGlobant20 Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

reddyashish Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

QilongTang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RobertGlobant20 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reddyashish commented Aug 9, 2023

Uh oh!

QilongTang commented Aug 9, 2023

Uh oh!

reddyashish Aug 9, 2023

Choose a reason for hiding this comment

Uh oh!

RobertGlobant20 Aug 10, 2023

Choose a reason for hiding this comment

Uh oh!

reddyashish Aug 11, 2023

Choose a reason for hiding this comment

Uh oh!

reddyashish Aug 9, 2023

Choose a reason for hiding this comment

Uh oh!

reddyashish commented Aug 9, 2023

Uh oh!

reddyashish commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QilongTang commented Aug 10, 2023

Uh oh!

QilongTang commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reddyashish commented Aug 10, 2023

Uh oh!

reddyashish commented Aug 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reddyashish commented Aug 1, 2023 •

edited

Loading

QilongTang left a comment •

edited

Loading

reddyashish commented Aug 10, 2023 •

edited

Loading

QilongTang commented Aug 10, 2023 •

edited

Loading