[Server Integration tests] Enrich integration GraphQL API tests #7699

gitstart-app · 2024-10-14T19:04:41Z

Description

We are using gql instead of strings to be able to see the graphql code highlighted

Demo

greptile-apps

PR Summary

This pull request enhances the integration tests for the GraphQL API, focusing on company-related operations. The changes introduce more comprehensive and organized testing strategies. Here are the key points:

Replaced single companies.integration-spec.ts file with multiple specialized test files for different company operations
Implemented GraphQL tag (gql) for improved query readability and syntax highlighting
Added tests for create, read, update, delete, and destroy operations for both single and multiple companies
Introduced utility functions in api-requests.ts and generate-record-name.ts for consistent API interaction and unique record naming
Enhanced teardown process in teardown-test.ts to clean up test data, ensuring test independence

These changes significantly improve the test coverage and maintainability of the company-related GraphQL API tests.

_{17 file(s) reviewed, 16 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-10-14T19:07:26Z

packages/twenty-server/test/companies/create-many-companies.integration-spec.ts

+`;
+
+describe('createCompaniesResolver (integration)', () => {
+  it('should create and return  companies', async () => {


syntax: Extra space in test description

greptile-apps · 2024-10-14T19:07:52Z

packages/twenty-server/test/companies/create-one-company.integration-spec.ts

+      expect(createdCompany).toHaveProperty('name');
+      expect(createdCompany.name).toEqual(companyName);
+
+      expect(createdCompany).toHaveProperty('employees');
+      expect(createdCompany).toHaveProperty('idealCustomerProfile');
+      expect(createdCompany).toHaveProperty('position');
+      expect(createdCompany).toHaveProperty('id');
+      expect(createdCompany).toHaveProperty('createdAt');
+      expect(createdCompany).toHaveProperty('updatedAt');
+      expect(createdCompany).toHaveProperty('deletedAt');
+      expect(createdCompany).toHaveProperty('accountOwnerId');
+      expect(createdCompany).toHaveProperty('tagline');
+      expect(createdCompany).toHaveProperty('workPolicy');
+      expect(createdCompany).toHaveProperty('visaSponsorship');


style: Consider using a loop or helper function to reduce repetition in property checks

greptile-apps · 2024-10-14T19:08:41Z

packages/twenty-server/test/companies/delete-one-company.integration-spec.ts

+
+    const findCompanyResponse = await apiRequest(findCompanyQueryData);
+
+    expect(findCompanyResponse.body.data.company).toBeNull();


style: Check for specific error message or code instead of just null

greptile-apps · 2024-10-14T19:09:09Z

packages/twenty-server/test/companies/destroy-many-companies.integration-spec.ts

+    expect(
+      findCompaniesResponse.body.data.companies.edges.filter((c) =>
+        companiesIds.includes(c.node.id),
+      ),
+    ).toHaveLength(0);


style: Use a more specific assertion

greptile-apps · 2024-10-14T19:09:09Z

packages/twenty-server/test/companies/destroy-many-companies.integration-spec.ts

+    const findDeletedCompaniesResponse = await apiRequest(
+      findDeletedCompaniesQueryData,
+    );


style: Use successfulApiRequest for consistency

greptile-apps · 2024-10-14T19:11:50Z

packages/twenty-server/test/companies/person-company-relation.integration-spec.ts

+    expect(findPersonResponse.body.data.person).toEqual(
+      expect.objectContaining({
+        company: {
+          id: companyId,
+          name: companyName,
+        },
+      }),
+    );


style: Consider using more specific assertions for nested objects

greptile-apps · 2024-10-14T19:12:42Z

packages/twenty-server/test/companies/update-one-company.integration-spec.ts

+    successfulApiRequest(updateCompanyQueryData).expect((res) => {
+      const updatedCompany = res.body.data.updateCompany;
+
+      expect(updatedCompany).toHaveProperty('name');
+      expect(updatedCompany.name).toEqual(companyName);
+    });


logic: Add await to ensure the request is completed before moving to the next step

greptile-apps · 2024-10-14T19:12:42Z

packages/twenty-server/test/companies/update-one-company.integration-spec.ts

+    successfulApiRequest(findCompanyQueryData).expect((res) => {
+      const company = res.body.data.company;
+
+      expect(company).toHaveProperty('name');
+      expect(company.name).toEqual(companyName);
+    });


logic: Add await to ensure the request is completed before the test ends

greptile-apps · 2024-10-14T19:14:36Z

packages/twenty-server/test/utils/teardown-test.ts

+// eslint-disable-next-line no-restricted-imports
+import jestConfig from '../../jest-integration.config';


style: Consider moving this import to the top of the file with other imports for consistency

greptile-apps · 2024-10-14T19:14:36Z

packages/twenty-server/test/utils/teardown-test.ts

+  apiRequest({
+    query,
+    variables: {
+      nameFilter,
+      personFilter,
+    },
+  }).then((res) => {
+    if (res.body.errors) throw new Error(JSON.stringify(res.body.errors));
+    global.app.close();
+  });


style: This apiRequest is using a .then() chain. Consider using async/await for better readability and error handling

charlesBochet · 2024-10-15T12:24:55Z

packages/twenty-server/jest-integration.config.ts

@@ -16,6 +16,7 @@ const jestConfig: JestConfigWithTsJest = {
  testTimeout: 15000,
  moduleNameMapper: {
    ...pathsToModuleNameMapper(tsConfig.compilerOptions.paths),
+    '^test/(.*)$': '<rootDir>/test/$1',


@gitstart-twenty what is this line about?

This fixes the import for ultils folder, Jest was not resolving the imports

charlesBochet · 2024-10-15T12:26:18Z

packages/twenty-server/test/companies/create-many-companies.integration-spec.ts

+  }
+`;
+
+describe('createCompaniesResolver (integration)', () => {


@gitstart-twenty this is will create a separate suite for the create-many usecase

Could we instead have only 1 suite for the whole company tests? We would still like to split the tests in multiple files but only have one suite. This will ensure in which order the tests are executed because they have side effects on each other

charlesBochet · 2024-10-15T12:26:47Z

packages/twenty-server/test/utils/api-requests.ts

+
+type StandardObjectsSingularName = 'Company' | 'Person';
+
+export const apiRequest = (data: QueryData) => {


makeGraphqlAPIRequest

charlesBochet · 2024-10-15T12:27:06Z

packages/twenty-server/test/utils/api-requests.ts

+    .send({ query: print(data.query), variables: data.variables || {} });
+};
+
+export const successfulApiRequest = (data: QueryData) => {


exectSuccessfullGraphqlAPIRequest

charlesBochet · 2024-10-15T12:28:03Z

packages/twenty-server/test/utils/teardown-test.ts

+// eslint-disable-next-line no-restricted-imports
+import jestConfig from '../../jest-integration.config';
+
+const query = gql`


Let's avoid that and have the suite clean itself. This will likely impact too much the performance overall

gitstart-twenty · 2024-10-15T13:26:57Z

Hello @charlesBochet
for these two comments below, we should use one file for all tests, and run the database clean-up on the afterAll hook, is this correct?

Could we instead have only 1 suite for the whole company tests? We would still like to split the tests in multiple files but only have one suite. This will ensure in which order the tests are executed because they have side effects on each other

Let's avoid that and have the suite clean itself. This will likely impact too much the performance overall

charlesBochet · 2024-10-15T16:24:56Z

@gitstart-twenty the idea is that we don't even need to cleanUp as the suite should clean itself through the different tests

So:

remove setUp, clean post and pre-suite (we only want a global setUp and tearDown which is already in place)
have only one suite (.spec), but split its content in several files (maybe the queries and response could be in different files or the content of the test could be in another file BUT they should not be .spec file)
follow the scenario in the main issue that should leave the database data in the same state as before the tests

Other than that the PR looks promising :)

gitstart-twenty · 2024-10-15T16:39:11Z

@charlesBochet thanks for the clarifications, just one question

remove setUp, clean post and pre-suite (we only want a global setUp and tearDown which is already in place)

so we don't need to change the teardown file, right?

charlesBochet · 2024-10-16T15:22:25Z

@gitstart-twenty Indeed, I think you can remove your changes in tearDown

gitstart-twenty · 2024-10-16T17:18:25Z

Hey @charlesBochet
I see that you moved some files and also already updated the teardown, do you need any other change from us?

gitstart-twenty · 2024-10-16T17:34:59Z

@charlesBochet
Actually, We noticed that in your changes you removed the expectSuccessfullGraphqlAPIRequest function but you are still using the function in the tests, are you currently changing this?

Also, the test command is broken after the changes

gitstart-twenty · 2024-10-16T19:57:27Z

@charlesBochet we have a fix for the tests (including the functions and the order of the tests) but we are not sure if you are going to update the branch

charlesBochet

@gitstart-twenty, I have made changes to your PR:

introduced utils to compute query instead of hard-coding them
separated existing auto-generated findMany tests from the full resolver lists tests on only one object
reworked a bit the folder arch for more clarity

charlesBochet · 2024-10-16T15:21:41Z

packages/twenty-server/jest-integration.config.ts

@@ -16,6 +16,7 @@ const jestConfig: JestConfigWithTsJest = {
  testTimeout: 15000,
  moduleNameMapper: {
    ...pathsToModuleNameMapper(tsConfig.compilerOptions.paths),
+    '^test/(.*)$': '<rootDir>/test/$1',


github-actions · 2024-10-17T17:17:30Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5584:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5555:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5564:7)�[39m
danger-results://tmp/danger-results-9210185c.json

Generated by 🚫 dangerJS against 0228e1e

[Server Integration tests] Enrich integration GraphQL API tests

0b554d6

greptile-apps bot reviewed Oct 14, 2024

View reviewed changes

gitstart-twenty requested a review from ijreilly October 14, 2024 20:11

gitstart-twenty added the -PR: awaiting review label Oct 14, 2024

charlesBochet self-assigned this Oct 15, 2024

charlesBochet reviewed Oct 15, 2024

View reviewed changes

charlesBochet added -PR: awaiting author and removed -PR: awaiting review labels Oct 15, 2024

gitstart-twenty added 2 commits October 16, 2024 13:02

Merge branch 'main' into TWNTY-7526

4910415

move the tests to one test file, and rename functions

5c7a740

gitstart-twenty added -PR: awaiting review and removed -PR: awaiting author labels Oct 16, 2024

gitstart-twenty requested a review from charlesBochet October 16, 2024 13:20

Refactor graphql api integration testing

ac1807e

charlesBochet and others added 4 commits October 17, 2024 18:44

Improve tests

7efde63

Fix tests

66396ce

Fix

8393141

Merge branch 'main' into TWNTY-7526

8093074

charlesBochet approved these changes Oct 17, 2024

View reviewed changes

Fix linter

0228e1e

charlesBochet merged commit 58fd340 into main Oct 17, 2024
7 checks passed

charlesBochet deleted the TWNTY-7526 branch October 17, 2024 17:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Server Integration tests] Enrich integration GraphQL API tests #7699

[Server Integration tests] Enrich integration GraphQL API tests #7699

gitstart-app bot commented Oct 14, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

greptile-apps bot Oct 14, 2024

charlesBochet Oct 15, 2024

gitstart-twenty Oct 15, 2024

charlesBochet Oct 16, 2024

charlesBochet Oct 15, 2024

charlesBochet Oct 15, 2024

charlesBochet Oct 15, 2024

charlesBochet Oct 15, 2024

gitstart-twenty commented Oct 15, 2024

charlesBochet commented Oct 15, 2024 •

edited

Loading

gitstart-twenty commented Oct 15, 2024 •

edited

Loading

charlesBochet commented Oct 16, 2024

gitstart-twenty commented Oct 16, 2024

gitstart-twenty commented Oct 16, 2024 •

edited

Loading

gitstart-twenty commented Oct 16, 2024

charlesBochet left a comment

charlesBochet Oct 16, 2024

github-actions bot commented Oct 17, 2024


		const findCompanyResponse = await apiRequest(findCompanyQueryData);

		expect(findCompanyResponse.body.data.company).toBeNull();

		// eslint-disable-next-line no-restricted-imports
		import jestConfig from '../../jest-integration.config';


		type StandardObjectsSingularName = 'Company' \| 'Person';

		export const apiRequest = (data: QueryData) => {

[Server Integration tests] Enrich integration GraphQL API tests #7699

[Server Integration tests] Enrich integration GraphQL API tests #7699

Conversation

gitstart-app bot commented Oct 14, 2024 • edited Loading

Description

Demo

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitstart-twenty commented Oct 15, 2024

charlesBochet commented Oct 15, 2024 • edited Loading

gitstart-twenty commented Oct 15, 2024 • edited Loading

charlesBochet commented Oct 16, 2024

gitstart-twenty commented Oct 16, 2024

gitstart-twenty commented Oct 16, 2024 • edited Loading

gitstart-twenty commented Oct 16, 2024

charlesBochet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2024

Log

gitstart-app bot commented Oct 14, 2024 •

edited

Loading

charlesBochet commented Oct 15, 2024 •

edited

Loading

gitstart-twenty commented Oct 15, 2024 •

edited

Loading

gitstart-twenty commented Oct 16, 2024 •

edited

Loading