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

CSHARP-4420: EG tests for Atlas Search #1001

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
facf8a1
- Search text operator
BorisDog Nov 18, 2022
0c9e2a6
CSHARP-4440: Incorporate MongoDB.Labs.Search library
BorisDog Dec 12, 2022
7bcf7c3
- Tests fix
BorisDog Dec 12, 2022
bd3bb51
Hide aws sdk package. (#981)
DmitryLukyanov Dec 5, 2022
332b09a
Revert "Hide aws sdk package. (#981)" (#982)
DmitryLukyanov Dec 6, 2022
ad74cdf
CSHARP-4428: LINQ3 not handling down cast in UpdateDefinitionBuilder …
rstam Dec 7, 2022
ff4c396
CSHARP-4421: Unify all spec tests in single test project (#977)
BorisDog Dec 8, 2022
cae2582
CSHARP-4427: Negative array indexes should throw ExpressionNotSupport…
rstam Dec 8, 2022
20bc1eb
CSHARP-4399: Remove use of activate_venv.sh and utils.sh. (#983)
DmitryLukyanov Dec 8, 2022
40b011d
CSHARP-4400: Upgrade xunit to 2.4.2 and remove SkippableFact package …
BorisDog Dec 12, 2022
0daabd7
CSHARP-4446: MongoQueryable methods should validate that mandatory ar…
rstam Dec 8, 2022
4d81692
CSHARP-4339: Added database.AsQueryable().
rstam Dec 6, 2022
43d8939
CSHARP-4339: Add support for $documents in LINQ.
rstam Dec 14, 2022
5ce4903
CSHARP-4358: Update tests that only test against LINQ2 to test agains…
rstam Dec 10, 2022
cc9a895
CSHARP-4445: Support 64-bit values for Skip and Limit/Take.
rstam Dec 13, 2022
586b2cc
Fix compilation errors resulting from merge.
rstam Dec 14, 2022
f159956
CSHARP-4255: Automatically create Queryable Encryption keys. (#961)
DmitryLukyanov Dec 14, 2022
454f37c
CSHARP-4255: Fix merge conflict. (#992)
DmitryLukyanov Dec 14, 2022
b3ca622
CSHARP-4455: Fix test assertation. (#994)
DmitryLukyanov Dec 17, 2022
cb8a4be
CSHARP-4420: EG tests for Atlas Search
BorisDog Dec 22, 2022
150dd8a
- PR comments
BorisDog Jan 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions build.cake
Expand Up @@ -240,6 +240,13 @@ Task("TestAtlasDataLake")
action: (BuildConfig buildConfig, Path testProject) =>
RunTests(buildConfig, testProject, filter: "Category=\"AtlasDataLake\""));

Task("TestAtlasSearch")
.IsDependentOn("Build")
.DoesForEach(
items: GetFiles("./**/MongoDB.Driver.Tests.csproj"),
action: (BuildConfig buildConfig, Path testProject) =>
RunTests(buildConfig, testProject, filter: "Category=\"AtlasSearch\""));

Task("TestOcsp")
.IsDependentOn("Build")
.DoesForEach(
Expand Down
20 changes: 20 additions & 0 deletions evergreen/evergreen.yml
Expand Up @@ -684,6 +684,15 @@ functions:
${PREPARE_SHELL}
evergreen/run-atlas-data-lake-test.sh

run-atlas-search-test:
- command: shell.exec
type: test
params:
working_dir: mongo-csharp-driver
script: |
${PREPARE_SHELL}
ATLAS_SEARCH="${ATLAS_SEARCH}" evergreen/run-atlas-search-test.sh

run-ocsp-test:
- command: shell.exec
type: test
Expand Down Expand Up @@ -1202,6 +1211,10 @@ tasks:
- func: bootstrap-mongohoused
- func: run-atlas-data-lake-test

- name: atlas-search-test
commands:
- func: run-atlas-search-test

- name: test-serverless-net472
exec_timeout_secs: 2700 # 45 minutes: 15 for setup + 30 for tests
commands:
Expand Down Expand Up @@ -2068,6 +2081,13 @@ buildvariants:
tasks:
- name: atlas-data-lake-test

- name: atlas-search-test
display_name: "Atlas Search Tests"
run_on:
- windows-64-vs2017-test
tasks:
- name: atlas-search-test

- name: gssapi-auth-tests-windows
run_on:
- windows-64-vs2017-test
Expand Down
14 changes: 14 additions & 0 deletions evergreen/run-atlas-search-test.sh
@@ -0,0 +1,14 @@
#!/usr/bin/env bash

set -o xtrace
set -o errexit # Exit the script with error if any of the commands fail

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a section like in most of other files about output variables: https://github.com/mongodb/mongo-csharp-driver/blob/master/evergreen/run-csfle-azure-tests.sh#L11

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


############################################
# Main Program #
############################################

echo "Running Atlas Search driver tests"

export ATLAS_SEARCH_TESTS_ENABLED=true

powershell.exe .\\build.ps1 --target TestAtlasSearch

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have plans for non windows envs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choice for Windows platform is arbitrary, but we don't plan to extend to other OS.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, I wrote it for 2 reasons:

  1. Better to use target settings approach like: --target=$TARGET that works on both windows and non windows machines
  2. powershell is only windows tool, so given you don't have validation on OS, potentially it might throw, but it's minor, see whether you want to change anything here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine for now, just copied from run-atlas-data-lake-test and run-atlas-connectivity-tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

James told me once that we had to use --target=TestAtlasSearch.

I.e. that the = was mandatory.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's mandatory only on non windows OSs if I recall correctly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At one point it was mandatory on my Windows machine also, so I got used to using the = form.

My guess is that at some point a Cake release made it mandatory, and they got lots of complaints, and then they reverted the change.

I'm assuming it's Cake that parses this argument(s) and not Powershell.

1 change: 1 addition & 0 deletions src/MongoDB.Driver/Linq/MongoQueryable.cs
Expand Up @@ -21,6 +21,7 @@
using System.Threading;
using System.Threading.Tasks;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Core.Misc;
using MongoDB.Driver.Search;

namespace MongoDB.Driver.Linq
Expand Down
79 changes: 79 additions & 0 deletions src/MongoDB.Driver/Search/Highlight.cs
@@ -0,0 +1,79 @@
// Copyright 2010-present MongoDB Inc.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyright has a bit different commenting format, see other files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I'll fix this in the main search PR too.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure whether it's already should be fixed, but if so - not fixed yet

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the future :( I would suggest using a different PR approach (we already did similar work in past) since it's hard to follow what was fixed and what should be reviewed. Sounds like at least in this file the above suggestion was applied..

//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

using MongoDB.Bson;
using MongoDB.Bson.Serialization.Attributes;

namespace MongoDB.Driver.Search
{
/// <summary>
/// Represents a result of highlighting.
/// </summary>
public sealed class Highlight
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this new file in this PR? Shouldn't all work that is not testing related be in the CSHARP-4440 PR?

In any case, I'm not reviewing this file in this PR because it's not testing related.

This whole folder is wildly out of date with CSHRP-4440.

{
/// <summary>
/// Gets or sets the document field which returned a match.
/// </summary>
[BsonElement("path")]
public string Path { get; private set; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why private set is here? Sounds like it does nothing?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds like it's for deserialization only. Does it deserve a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's for deserialization. Looks pretty minor for a comment.


/// <summary>
/// Gets or sets one or more objects containing the matching text and the surrounding text
/// (if any).
/// </summary>
[BsonElement("texts")]
public HighlightText[] Texts { get; private set; }

/// <summary>
/// Gets or sets the score assigned to this result.
/// </summary>
[BsonElement("score")]
public double Score { get; private set; }
}

/// <summary>
/// Represents the matching text or the surrounding text of a highlighting result.
/// </summary>
public class HighlightText
{
/// <summary>
/// Gets or sets the text from the field which returned a match.
/// </summary>
[BsonElement("value")]
public string Value { get; private set; }

/// <summary>
/// Gets or sets the type of text, matching or surrounding.
/// </summary>
[BsonElement("type")]
[BsonRepresentation(BsonType.String)]
public HighlightTextType Type { get; private set; }
}

/// <summary>
/// Represents the type of text in a highlighting result, matching or surrounding.
/// </summary>
public enum HighlightTextType
{
/// <summary>
/// Indicates that the text contains a match.
/// </summary>
Hit,

/// <summary>
/// Indicates that the text contains the text content adjacent to a matching string.
/// </summary>
Text
}
}
23 changes: 23 additions & 0 deletions src/MongoDB.Driver/Search/HighlightOptions.cs
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using System.Linq.Expressions;
using MongoDB.Bson;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Core.Misc;
Expand Down Expand Up @@ -42,6 +44,27 @@ public HighlightOptions(PathDefinition<TDocument> path, int? maxCharsToExamine =
_maxNumPassages = maxNumPassages;
}

/// <summary>
/// Creates highlighting options.
/// </summary>
/// <param name="path">The document field to search.</param>
/// <param name="maxCharsToExamine">
/// The maximum number of characters to examine on a document when performing highlighting
/// for a field.
/// </param>
/// <param name="maxNumPassages">
/// The number of high-scoring passages to return per document in the highlighting results
/// for each field.
/// </param>
/// <returns>Highlighting options.</returns>
public HighlightOptions(
Expression<Func<TDocument, object>> path,
int? maxCharsToExamine = null,
int? maxNumPassages = null) :
this(new ExpressionFieldDefinition<TDocument>(path), maxCharsToExamine, maxNumPassages)
{
}

/// <summary>
/// Gets or sets the document field to search.
/// </summary>
Expand Down