Unverified Commit 084c9c18 authored by Nick Craver's avatar Nick Craver Committed by GitHub

NRediSearch Test Suite Fixes (#1383)

### NRediSearch
- Updated module so we're testing the right thing
- More output in the logging path for diagnosing issues
- Remove all the `FLUSHDB` calls masking issues
- Handle duplicate documents in latest module

Note: **locally** (on WSL 2), this fixes some issues and introduces stability but *does not* fix 2 scoring issues. Suggestions are coming back with scores > 1.0, which by the documentation is not supposed to happen...so I'm not sure what should be happening here. Here's the local WSL fail:
```
 NRediSearch.Test.ClientTests.ClientTest.TestGetSuggestionWithScore:
  Message: 
    StackExchange.Redis.RedisCommandException : Missing required fields:  score not within range: 2.11660146713257
  Stack Trace: 
    SuggestionBuilder.Build() line 99
    Client.GetSuggestionsWithPayloadAndScores(RedisResult[] results) line 1271
    Client.GetSuggestions(String prefix, SuggestionOptions options) line 796
    ClientTest.TestGetSuggestionWithScore() line 701
```
```
NRediSearch.Test.ClientTests.ClientTest.TestAddSuggestionGetSuggestionPayloadScores:
  Message: 
    StackExchange.Redis.RedisCommandException : Missing required fields:  score not within range: 2.71068739891052
  Stack Trace: 
    SuggestionBuilder.Build() line 99
    Client.GetSuggestionsWithPayloadAndScores(RedisResult[] results) line 1271
    Client.GetSuggestions(String prefix, SuggestionOptions options) line 796
    ClientTest.TestAddSuggestionGetSuggestionPayloadScores() line 642
```

However, the current fixes tidy up the CI pipeline so I recommend merging this as-is, then I'll keep following up with various local errors and I'm looking at some Docker test options as alternatives to WSL2 so that people can run either.
parent 29ee2152
...@@ -52,6 +52,8 @@ public enum IndexOptions ...@@ -52,6 +52,8 @@ public enum IndexOptions
public sealed class ConfiguredIndexOptions public sealed class ConfiguredIndexOptions
{ {
// This news up a enum which results in the 0 equivalent.
// It's not used in the library and I'm guessing this isn't intentional.
public static IndexOptions Default => new IndexOptions(); public static IndexOptions Default => new IndexOptions();
private IndexOptions _options; private IndexOptions _options;
...@@ -320,7 +322,7 @@ public bool AddDocument(Document doc, AddOptions options = null) ...@@ -320,7 +322,7 @@ public bool AddDocument(Document doc, AddOptions options = null)
{ {
return (string)DbSync.Execute("FT.ADD", args) == "OK"; return (string)DbSync.Execute("FT.ADD", args) == "OK";
} }
catch (RedisServerException ex) when (ex.Message == "Document already in index") catch (RedisServerException ex) when (ex.Message == "Document already in index" || ex.Message == "Document already exists")
{ {
return false; return false;
} }
......
...@@ -96,7 +96,7 @@ public Suggestion Build() ...@@ -96,7 +96,7 @@ public Suggestion Build()
if (isStringMissing || isScoreOutOfRange) if (isStringMissing || isScoreOutOfRange)
{ {
throw new RedisCommandException($"Missing required fields: {(isStringMissing ? "string" : string.Empty)} {(isScoreOutOfRange ? "score not within range" : string.Empty)}"); throw new RedisCommandException($"Missing required fields: {(isStringMissing ? "string" : string.Empty)} {(isScoreOutOfRange ? "score not within range" : string.Empty)}: {_score.ToString()}");
} }
return new Suggestion(this); return new Suggestion(this);
......
...@@ -50,9 +50,8 @@ public void Search() ...@@ -50,9 +50,8 @@ public void Search()
Assert.True(cl.DropIndex()); Assert.True(cl.DropIndex());
var ex = Assert.Throws<RedisServerException>(() => cl.Search(new Query("hello world"))); var ex = Assert.Throws<RedisServerException>(() => cl.Search(new Query("hello world")));
Assert.True( Output.WriteLine("Exception: " + ex.Message);
string.Equals("Unknown Index name", ex.Message, System.StringComparison.InvariantCultureIgnoreCase) Assert.True(IsMissingIndexException(ex));
|| string.Equals("no such index", ex.Message, System.StringComparison.InvariantCultureIgnoreCase));
} }
[Fact] [Fact]
...@@ -311,7 +310,6 @@ public void TestAddHash() ...@@ -311,7 +310,6 @@ public void TestAddHash()
public void TestDrop() public void TestDrop()
{ {
Client cl = GetClient(); Client cl = GetClient();
Db.Execute("FLUSHDB"); // yeah, this is horrible, deal with it
Schema sc = new Schema().AddTextField("title", 1.0); Schema sc = new Schema().AddTextField("title", 1.0);
...@@ -329,25 +327,27 @@ public void TestDrop() ...@@ -329,25 +327,27 @@ public void TestDrop()
Assert.Equal(100, res.TotalResults); Assert.Equal(100, res.TotalResults);
var key = (string)Db.KeyRandom(); var key = (string)Db.KeyRandom();
Output.WriteLine("Found key: " + key);
Assert.NotNull(key); Assert.NotNull(key);
Reset(cl); Reset(cl);
key = (string)Db.KeyRandom(); var indexExists = Db.KeyExists(cl.IndexName);
Assert.Null(key); Assert.False(indexExists);
} }
[Fact] [Fact]
public void TestAlterAdd() public void TestAlterAdd()
{ {
Client cl = GetClient(); Client cl = GetClient();
Db.Execute("FLUSHDB"); // YEAH, this is still horrible and I'm still dealing with it.
Schema sc = new Schema().AddTextField("title", 1.0); Schema sc = new Schema().AddTextField("title", 1.0);
Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions())); Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions()));
var fields = new Dictionary<string, RedisValue>(); var fields = new Dictionary<string, RedisValue>
fields.Add("title", "hello world"); {
{ "title", "hello world" }
};
for (int i = 0; i < 100; i++) for (int i = 0; i < 100; i++)
{ {
Assert.True(cl.AddDocument($"doc{i}", fields)); Assert.True(cl.AddDocument($"doc{i}", fields));
...@@ -559,9 +559,7 @@ public void TestDropMissing() ...@@ -559,9 +559,7 @@ public void TestDropMissing()
{ {
Client cl = GetClient(); Client cl = GetClient();
var ex = Assert.Throws<RedisServerException>(() => cl.DropIndex()); var ex = Assert.Throws<RedisServerException>(() => cl.DropIndex());
Assert.True( Assert.True(IsMissingIndexException(ex));
string.Equals("Unknown Index name", ex.Message, System.StringComparison.InvariantCultureIgnoreCase)
|| string.Equals("no such index", ex.Message, System.StringComparison.InvariantCultureIgnoreCase));
} }
[Fact] [Fact]
...@@ -582,7 +580,6 @@ public void TestGet() ...@@ -582,7 +580,6 @@ public void TestGet()
public void TestMGet() public void TestMGet()
{ {
Client cl = GetClient(); Client cl = GetClient();
Db.Execute("FLUSHDB"); // YEAH, this is still horrible and I'm still dealing with it.
cl.CreateIndex(new Schema().AddTextField("txt1", 1.0), new ConfiguredIndexOptions()); cl.CreateIndex(new Schema().AddTextField("txt1", 1.0), new ConfiguredIndexOptions());
cl.AddDocument(new Document("doc1").Set("txt1", "Hello World!1"), new AddOptions()); cl.AddDocument(new Document("doc1").Set("txt1", "Hello World!1"), new AddOptions());
...@@ -726,6 +723,10 @@ public void TestGetTagField() ...@@ -726,6 +723,10 @@ public void TestGetTagField()
Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions())); Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions()));
var search = cl.Search(new Query("hello"));
Output.WriteLine("Initial search: " + search.TotalResults);
Assert.Equal(0, search.TotalResults);
var fields1 = new Dictionary<string, RedisValue>(); var fields1 = new Dictionary<string, RedisValue>();
fields1.Add("title", "hello world"); fields1.Add("title", "hello world");
fields1.Add("category", "red"); fields1.Add("category", "red");
...@@ -750,7 +751,13 @@ public void TestGetTagField() ...@@ -750,7 +751,13 @@ public void TestGetTagField()
Assert.Equal(1, cl.Search(new Query("@category:{yellow}")).TotalResults); Assert.Equal(1, cl.Search(new Query("@category:{yellow}")).TotalResults);
Assert.Equal(0, cl.Search(new Query("@category:{purple}")).TotalResults); Assert.Equal(0, cl.Search(new Query("@category:{purple}")).TotalResults);
Assert.Equal(1, cl.Search(new Query("@category:{orange\\;purple}")).TotalResults); Assert.Equal(1, cl.Search(new Query("@category:{orange\\;purple}")).TotalResults);
Assert.Equal(4, cl.Search(new Query("hello")).TotalResults); search = cl.Search(new Query("hello"));
Output.WriteLine("Post-search: " + search.TotalResults);
foreach (var doc in search.Documents)
{
Output.WriteLine("Found: " + doc.Id);
}
Assert.Equal(4, search.TotalResults);
} }
[Fact] [Fact]
...@@ -818,7 +825,6 @@ public void TestMultiDocuments() ...@@ -818,7 +825,6 @@ public void TestMultiDocuments()
public void TestReturnFields() public void TestReturnFields()
{ {
Client cl = GetClient(); Client cl = GetClient();
Db.Execute("FLUSHDB");
Schema sc = new Schema().AddTextField("field1", 1.0).AddTextField("field2", 1.0); Schema sc = new Schema().AddTextField("field1", 1.0).AddTextField("field2", 1.0);
Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions())); Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions()));
...@@ -841,7 +847,6 @@ public void TestReturnFields() ...@@ -841,7 +847,6 @@ public void TestReturnFields()
public void TestInKeys() public void TestInKeys()
{ {
Client cl = GetClient(); Client cl = GetClient();
Db.Execute("FLUSHDB");
Schema sc = new Schema().AddTextField("field1", 1.0).AddTextField("field2", 1.0); Schema sc = new Schema().AddTextField("field1", 1.0).AddTextField("field2", 1.0);
Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions())); Assert.True(cl.CreateIndex(sc, new ConfiguredIndexOptions()));
......
...@@ -27,28 +27,52 @@ public void Dispose() ...@@ -27,28 +27,52 @@ public void Dispose()
Db = null; Db = null;
} }
protected Client GetClient([CallerMemberName] string caller = null) protected Client GetClient([CallerFilePath] string filePath = null, [CallerMemberName] string caller = null)
=> Reset(new Client(GetType().Name + ":" + caller, Db)); {
// Remove all that extra pathing
var offset = filePath?.IndexOf("NRediSearch.Test");
if (offset > -1)
{
filePath = filePath.Substring(offset.Value + "NRediSearch.Test".Length + 1);
}
var indexName = $"{filePath}:{caller}";
Output.WriteLine("Using Index: " + indexName);
var exists = Db.KeyExists("idx:" + indexName);
Output.WriteLine("Key existed: " + exists);
protected static Client Reset(Client client) var client = new Client(indexName, Db);
var wasReset = Reset(client);
Output.WriteLine("Index was reset?: " + wasReset);
return client;
}
protected bool Reset(Client client)
{ {
Output.WriteLine("Resetting index");
try try
{ {
client.DropIndex(); // tests create them var result = client.DropIndex(); // tests create them
Output.WriteLine(" Result: " + result);
return result;
} }
catch (RedisServerException ex) catch (RedisServerException ex)
{ {
if (string.Equals("Unknown Index name", ex.Message, System.StringComparison.InvariantCultureIgnoreCase) if (string.Equals("Unknown Index name", ex.Message, StringComparison.InvariantCultureIgnoreCase))
|| string.Equals("no such index", ex.Message, System.StringComparison.InvariantCultureIgnoreCase))
{ {
// fine Output.WriteLine(" Unknown index name");
return true;
}
if (string.Equals("no such index", ex.Message, StringComparison.InvariantCultureIgnoreCase))
{
Output.WriteLine(" No such index");
return true;
} }
else else
{ {
throw; throw;
} }
} }
return client;
} }
internal static ConnectionMultiplexer GetWithFT(ITestOutputHelper output) internal static ConnectionMultiplexer GetWithFT(ITestOutputHelper output)
...@@ -117,5 +141,15 @@ internal static ConnectionMultiplexer GetWithFT(ITestOutputHelper output) ...@@ -117,5 +141,15 @@ internal static ConnectionMultiplexer GetWithFT(ITestOutputHelper output)
} }
return data; return data;
} }
protected bool IsMissingIndexException(Exception ex)
{
if (ex.Message == null)
{
return false;
}
return ex.Message.Contains("Unknown Index name", StringComparison.InvariantCultureIgnoreCase)
|| ex.Message.Contains("no such index", StringComparison.InvariantCultureIgnoreCase);
}
} }
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment