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.

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

chore: Add testDescription helper and update error processing #1

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: Add testDescription helper and update error processing #1

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89046b2
chore: Add testDescription helper and error processing
asararatnakar Sep 17, 2024
9d88c4d
format
asararatnakar Sep 17, 2024
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
38 changes: 23 additions & 15 deletions packages/gqltest/gqltest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const chai = require("chai");
const chaiGraphQL = require("chai-graphql");
chai.use(chaiGraphQL);

const {introspectionTests} = require("./_introspection.js");
const { introspectionTests } = require("./_introspection.js");

// GQLHeaders holds headers for a request.
//
Expand All @@ -25,7 +25,7 @@ class GQLHeaders {
"Content-Type": "application/json",
});
if (process.env.GQLTEST_HEADERS) {
for (let [name,value] of Object.entries(JSON.parse(process.env.GQLTEST_HEADERS))) {
for (let [name, value] of Object.entries(JSON.parse(process.env.GQLTEST_HEADERS))) {
this.headers.set(name, value);
}
}
Expand Down Expand Up @@ -61,10 +61,23 @@ class GQLResponse {
// Asserts that the GraphQL response has status 200 and no errors.
GQLResponse.prototype.expectOK = function () {
chai.expect(this.response.status).to.equal(200);
chai.assert.notGraphQLError(this.body);
return this;
};


// defaultTimeout can be used in describe.timeout()
// for requests against a graphql endpoint.
// Match stepzen AWS 60 second timeout.
const defaultTimeout = 60000
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably ok for now, but gqltest was trying to stay GraphQL neutral (hence stepzen.js). They may need to be some thought on how to handle default timeouts nicely.


function testDescription(testRoot, fullDirName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not yet convinced this is suitable for a generic test harness, seems clearer and more typical just to have constants for describe / it.

Maybe move to a separate PR.

segments = fullDirName.split(path.sep)
rootIndex = segments.findIndex(element => element == testRoot)
// Construct the test description from the unique path from testRoot, which is likely the root of the git repo.
// Intentionally not using `path.sep` as this is not a path to a file now, but a test description.
return segments.slice(rootIndex + 1, -1).join('/')
}

async function _execute({
test,
endpoint,
Expand Down Expand Up @@ -132,8 +145,8 @@ async function execute({
//
// (1) value rooted at `data`: {customer: {name: "Fred"}}
// (2) root value with no errors: {data: {customer: {name: "Fred"}}}
// (3) not implemented - root value with field errors: {data: {customer: {name: "Fred" email:null}}, "errors":[...]}
// (4) not implemented - root value with request errors: {"errors":[...]}
// (3) root value with field errors: {data: {customer: {name: "Fred" email:null}}, "errors":[...]}
// or request errors: {"errors":[...]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Responses with request errors are different to responses with field errors, which is why they were separated. Please keep the separation.

//
// Workarounds for (1) if "data" or "errors" are the root fields under "data" in a response:
// - use approach (2)
Expand All @@ -142,19 +155,11 @@ function assertExpected(response, expected, label) {
expected = optionalJSONFromFile(expected, label);

// (2),(3) - Response at the root.
if (Object.hasOwn(expected, "data")) {
if (Object.hasOwn(expected, "errors")) {
chai.expect.fail("field errors in response not yet supported.")
}
if (Object.hasOwn(expected, "data") || Object.hasOwn(expected, "errors")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just 2,3 means it's data && errors.

chai.expect(response.body).to.deep.equal(expected);
Copy link
Contributor

Choose a reason for hiding this comment

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

Generically this will not work for expected with errors, which is why it needs to be separated.
This is because the order of errors cannot be guaranteed. Expected field errors need to be explicitly tested that a matching error with the expected path exists. Thus need to test that data matches and then process expected errors.

return;
}

// (4) request errors
if (Object.hasOwn(expected, "errors")) {
chai.expect.fail("request errors in response not yet supported.")
}

// (1) - Non-error response rooted at data.
chai.assert.graphQL(response.body, expected);
}
Expand Down Expand Up @@ -184,7 +189,8 @@ async function runtests(label, endpoint, headers, tests) {
}

describe(label, function () {
beforeEach("test-info", function() {
this.timeout(defaultTimeout) // Occasional requests take > 2s
beforeEach("test-info", function () {
this.gql_title = this.currentTest.title;
})
afterEach("log-failure", logOnFail);
Expand Down Expand Up @@ -250,3 +256,5 @@ exports.GQLHeaders = GQLHeaders;
exports.GQLResponse = GQLResponse;
exports.logOnFail = logOnFail;
exports.introspectionTests = introspectionTests;
exports.defaultTimeout = defaultTimeout
exports.testDescription = testDescription;