Skip to content

Add unit tests and fix few issues #396

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.

Sign up for GitHub

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

Merged
tarazou9 merged 12 commits into from
May 11, 2022
Merged

Add unit tests and fix few issues #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3534a79
Add unit tests and fix a few issues
tarazou9 May 9, 2022
cd9138e
Add more assert
tarazou9 May 9, 2022
fe3eb55
Fix format
tarazou9 May 9, 2022
41e3925
Add tests for inner objects and fix an issue related to query with in…
tarazou9 May 10, 2022
15b6a03
Merge from master
tarazou9 May 10, 2022
b70f038
add named argument for tests
tarazou9 May 10, 2022
867d6ef
Check empty page for querying single item
tarazou9 May 11, 2022
94456c6
Fix formatting error
tarazou9 May 11, 2022
a4e31d8
Address PR comments
tarazou9 May 11, 2022
3654899
Merge branch 'main' into users/tarazou/unitTests
tarazou9 May 11, 2022
d7d96a7
Fix typo
tarazou9 May 11, 2022
d0346ba
Fix comment
tarazou9 May 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions DataGateway.Service.Tests/CosmosTests/MutationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public async Task CanCreateItemWithoutVariables()
name
}}
}}";
JsonElement response = await ExecuteGraphQLRequestAsync("addPlanet", mutation, new());
JsonElement response = await ExecuteGraphQLRequestAsync("addPlanet", mutation, variables: new());

// Validate results
Assert.AreEqual(id, response.GetProperty("id").GetString());
Expand All @@ -100,7 +100,7 @@ public async Task CanDeleteItemWithoutVariables()
name
}}
}}";
_ = await ExecuteGraphQLRequestAsync("addPlanet", addMutation, new());
_ = await ExecuteGraphQLRequestAsync("addPlanet", addMutation, variables: new());

// Run mutation delete item;
string deleteMutation = $@"
Expand All @@ -110,12 +110,43 @@ public async Task CanDeleteItemWithoutVariables()
name
}}
}}";
JsonElement response = await ExecuteGraphQLRequestAsync("deletePlanet", deleteMutation, new());
JsonElement response = await ExecuteGraphQLRequestAsync("deletePlanet", deleteMutation, variables: new());

// Validate results
Assert.IsNull(response.GetProperty("id").GetString());
}

[TestMethod]
public async Task MutationMissingInputReturnError()
{
// Run mutation Add planet without any input
string mutation = $@"
mutation {{
addPlanet {{
id
name
}}
}}";
JsonElement response = await ExecuteGraphQLRequestAsync("addPlanet", mutation, variables: new());
Assert.AreEqual("inputDict is missing", response[0].GetProperty("message").ToString());
}
Comment on lines +120 to +132

@aaronpowell aaronpowell May 10, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test should fail at the Hot Chocolate level, not at the runtime level, because the GraphQL is malformed. Should we be writing tests that aren't really run through the runtime pipeline?

@tarazou9 tarazou9 May 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@aaronpowell This GraphQL query is in actually a valid form and will run through the runtime process. Please see screenshot as below.
image

@aaronpowell aaronpowell May 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh interesting. I had a look at the schema and the reason is that both the arguments to the mutation are nullable, meaning that you can call the mutation with no arguments.

Admittedly this is only a problem with the design of the current schema and pipeline, so we'll review with #399


[TestMethod]
public async Task MutationMissingRequiredIdReturnError()
{
// Run mutation Add planet without id
const string name = "test_name";
string mutation = $@"
mutation {{
addPlanet ( name: ""{name}"") {{
id
name
}}
}}";
JsonElement response = await ExecuteGraphQLRequestAsync("addPlanet", mutation, variables: new());
Assert.AreEqual("id field is mandatory", response[0].GetProperty("message").ToString());
}

/// <summary>
/// Runs once after all tests in this class are executed
/// </summary>
Expand Down
122 changes: 121 additions & 1 deletion DataGateway.Service.Tests/CosmosTests/QueryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.Json;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down Expand Up @@ -69,21 +70,27 @@ public async Task GetPaginatedWithVariables()
// Run query
JsonElement response = await ExecuteGraphQLRequestAsync("planetList", PlanetListQuery);
int actualElements = response.GetArrayLength();
List<string> responseTotal = new();
ConvertJsonElementToStringList(response, responseTotal);

// Run paginated query
int totalElementsFromPaginatedQuery = 0;
string continuationToken = null;
const int pagesize = 5;
List<string> pagedResponse = new();

do
{
JsonElement page = await ExecuteGraphQLRequestAsync("planets", PlanetConnectionQueryStringFormat, new() { { "first", pagesize }, { "after", continuationToken } });
JsonElement continuation = page.GetProperty("endCursor");
continuationToken = continuation.ToString();
totalElementsFromPaginatedQuery += page.GetProperty("items").GetArrayLength();
ConvertJsonElementToStringList(page.GetProperty("items"), pagedResponse);
} while (!string.IsNullOrEmpty(continuationToken));

// Validate results
Assert.AreEqual(actualElements, totalElementsFromPaginatedQuery);
Assert.IsTrue(responseTotal.SequenceEqual(pagedResponse));
}

[TestMethod]
Expand All @@ -110,10 +117,14 @@ public async Task GetPaginatedWithoutVariables()
// Run query
JsonElement response = await ExecuteGraphQLRequestAsync("planetList", PlanetListQuery);
int actualElements = response.GetArrayLength();
List<string> responseTotal = new();
ConvertJsonElementToStringList(response, responseTotal);

// Run paginated query
int totalElementsFromPaginatedQuery = 0;
string continuationToken = null;
const int pagesize = 5;
List<string> pagedResponse = new();

do
{
Expand All @@ -129,14 +140,123 @@ public async Task GetPaginatedWithoutVariables()
}}
}}";

JsonElement page = await ExecuteGraphQLRequestAsync("planets", planetConnectionQueryStringFormat, new());
JsonElement page = await ExecuteGraphQLRequestAsync("planets", planetConnectionQueryStringFormat, variables: new());
JsonElement continuation = page.GetProperty("endCursor");
continuationToken = continuation.ToString();
totalElementsFromPaginatedQuery += page.GetProperty("items").GetArrayLength();
ConvertJsonElementToStringList(page.GetProperty("items"), pagedResponse);
} while (!string.IsNullOrEmpty(continuationToken));

// Validate results
Assert.AreEqual(actualElements, totalElementsFromPaginatedQuery);
Assert.IsTrue(responseTotal.SequenceEqual(pagedResponse));
}

/// <summary>
/// Query List Type with input parameters
/// </summary>
/// <returns></returns>
[TestMethod]
public async Task GetListTypeWithParameters()
{
string id = _idList[0];
string query = @$"
query {{
getPlanetListById (id: ""{id}"") {{
id
name
}}
}}";

JsonElement response = await ExecuteGraphQLRequestAsync("getPlanetListById", query);

// Validate results
Assert.AreEqual(1, response.GetArrayLength());
Assert.AreEqual(id, response[0].GetProperty("id").ToString());
}

/// <summary>
/// Query single item by non-primary key field, found no match
/// </summary>
/// <returns></returns>
[TestMethod]
public async Task GetByNonePrimaryFieldResultNotFound()
{
string name = "non-existed name";
string query = @$"
query {{
getPlanetByName (name: ""{name}"") {{
id
name
}}
}}";

JsonElement response = await ExecuteGraphQLRequestAsync("getPlanetByName", query);

// Validate results
Assert.IsNull(response.Deserialize<string>());
}

/// <summary>
/// Query single item by non-primary key field, found record back
/// </summary>
/// <returns></returns>
[TestMethod]
public async Task GetByNonPrimaryFieldReturnsResult()
{
string name = "test name";
string query = @$"
query {{
getPlanetByName (name: ""{name}"") {{
id
name
}}
}}";

JsonElement response = await ExecuteGraphQLRequestAsync("getPlanetByName", query);

// Validate results
Assert.AreEqual(name, response.GetProperty("name").ToString());
Comment thread
seantleonard marked this conversation as resolved.
}

/// <summary>
/// Query result with nested object
/// </summary>
/// <returns></returns>
[TestMethod]
public async Task GetByPrimaryKeyWithInnerObject()
{
// Run query
string id = _idList[0];
string query = @$"
query {{
planetById (id: ""{id}"") {{
id
name
character {{
id
name
}}
}}
}}";
JsonElement response = await ExecuteGraphQLRequestAsync("planetById", query);

// Validate results
Assert.AreEqual(id, response.GetProperty("id").GetString());
Comment thread
seantleonard marked this conversation as resolved.
}

private static void ConvertJsonElementToStringList(JsonElement ele, List<string> strList)
{
if (ele.ValueKind == JsonValueKind.Array)
{
JsonElement.ArrayEnumerator enumerator = ele.EnumerateArray();

while (enumerator.MoveNext())
{
JsonElement prop = enumerator.Current;
strList.Add(prop.ToString());
}
}
}

/// <summary>
Expand Down
13 changes: 10 additions & 3 deletions DataGateway.Service.Tests/CosmosTests/TestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.IO;
using System.Net.Http;
using System.Security.Claims;
using System.Text;
using System.Text.Json;
using System.Threading.Tasks;
Expand Down Expand Up @@ -42,6 +43,8 @@ type Query {
getPlanet(id: ID, name: String): Planet
planetList: [Planet]
planets(first: Int, after: String): PlanetConnection
getPlanetListById(id: ID): [Planet]
getPlanetByName(name: String): Planet
Comment on lines +46 to +47

@aaronpowell aaronpowell May 10, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These queries won't exist when #399 merges as we don't generate queries that match that signature, instead we use the typed filter (see https://github.com/Azure/project-hawaii/issues/11 for more on the typed filter).

Consider what will needed to be done to refactor the tests that use those queries, and whether it'd be better to write them now to use a typed filter, rather than prepare something for a rewrite later on.

@mbhaskar mbhaskar May 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thats a very good point Aaron. I think this test atleast is good with what we have already checked in today. May be we can just clean this up when 399 merges?

}

type Mutation {
Expand All @@ -65,7 +68,8 @@ type Character {

type Planet {
id : ID,
name : String
name : String,
character: Character
}";

_metadataStoreProvider.GraphQLSchema = jsonString;
Expand Down Expand Up @@ -109,9 +113,11 @@ private static DefaultHttpContext GetHttpContextWithBody(string data)
HttpRequestMessage request = new();
MemoryStream stream = new(Encoding.UTF8.GetBytes(data));
request.Method = HttpMethod.Post;
ClaimsPrincipal user = new(new ClaimsIdentity(authenticationType: "Bearer"));
DefaultHttpContext httpContext = new()
{
Request = { Body = stream, ContentLength = stream.Length }
Request = { Body = stream, ContentLength = stream.Length },
User = user
};
return httpContext;
}
Expand Down Expand Up @@ -182,7 +188,8 @@ internal static async Task<JsonElement> ExecuteGraphQLRequestAsync(string queryN

if (graphQLResult.TryGetProperty("errors", out JsonElement errors))
{
Assert.Fail(errors.GetRawText());
// to validate expected errors and error message
return errors;
}

return graphQLResult.GetProperty("data").GetProperty(queryName);
Expand Down
9 changes: 9 additions & 0 deletions DataGateway.Service.Tests/CosmosTests/TestHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public static object GetItem(string id)
return new
{
id = id,
name = "test name",
myProp = "a value",
myIntProp = 4,
myBooleanProp = true,
Expand All @@ -56,6 +57,14 @@ public static object GetItem(string id)
lastName = "the last name",
zipCode = 784298
}
},
character = new
{
id = id,
name = "planet character",
type = "Mars",
homePlanet = 1,
primaryFunction = "test function"
}
};
}
Expand Down
4 changes: 2 additions & 2 deletions DataGateway.Service/Resolvers/CosmosMutationEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ private async Task<JObject> ExecuteAsync(IDictionary<string, object?> inputDict,

JObject jObject;

if (inputDict != null)
if (inputDict != null && inputDict.Count > 0)
{
// TODO: optimize this multiple round of serialization/deserialization
string json = JsonConvert.SerializeObject(inputDict);
Expand Down Expand Up @@ -75,7 +75,7 @@ private async Task<JObject> ExecuteAsync(IDictionary<string, object?> inputDict,

break;
default:
throw new NotSupportedException($"unsupprted operation type: {resolver.OperationType.ToString()}");
throw new NotSupportedException($"unsupported operation type: {resolver.OperationType.ToString()}");

@aaronpowell aaronpowell May 10, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be converted to a DataGatewayException? Is there an appropriate status code combination to reflect this?

@mbhaskar mbhaskar May 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I dont think we are not using DataGatewayException for NotSupportedException as of now even in other areas of code. So this should be OK?

}

return response.Resource;
Expand Down
Loading