Workflow E2E tests – batch 2 #9747

Devessier · 2025-01-20T15:13:31Z

Fix the e2e according to the last changes in the workflows
Create a few more tests regarding the workflow visualizer

greptile-apps

PR Summary

This PR enhances workflow E2E testing capabilities with comprehensive test coverage for workflow creation, modification, and status management.

Added workflow trigger types (record-created, record-updated, etc.) and action types (create-record, update-record, etc.) in blank-workflow.ts for standardized testing
Implemented robust step management methods (createStep, deleteStep) with response validation and error handling
Added three comprehensive E2E tests covering workflow creation, step deletion persistence, and active workflow modification
Enhanced test selection capabilities by adding data-testid attribute to workflow status container
Introduced proper test cleanup with workflow deletion after each test run

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

greptile-apps · 2025-01-20T15:14:34Z

packages/twenty-e2e-testing/lib/fixtures/blank-workflow.ts

+
+    await stepNode.click();
+
+    await Promise.all([


logic: Potential race condition in deleteStep between visibility check and actual deletion. Node might become invisible for other reasons.

greptile-apps · 2025-01-20T15:14:35Z

packages/twenty-e2e-testing/tests/workflow-visualizer.spec.ts

+  await Promise.all([
+    expect(workflowVisualizer.workflowStatus).toHaveText('Active'),
+
+    workflowVisualizer.activateWorkflowButton.click(),
+  ]);


logic: Race condition risk - activateWorkflowButton.click() should be awaited before checking workflowStatus. The Promise.all here could lead to flaky tests.

greptile-apps · 2025-01-20T15:14:36Z

packages/twenty-e2e-testing/tests/workflow-visualizer.spec.ts

+  await Promise.all([
+    page.reload(),
+
+    expect(workflowVisualizer.triggerNode).toBeVisible(),
+  ]);


logic: Potential race condition between page reload and trigger node visibility check. Consider awaiting page load event first.

prastoin

That's a really nice refactor and POM implem

prastoin · 2025-01-20T15:33:02Z

packages/twenty-e2e-testing/tests/workflow-visualizer.spec.ts

+
+  await workflowVisualizer.deleteStep(fifthStepId);
+
+  await expect(workflowVisualizer.getAllStepNodes()).toContainText([


Remark: This might be HA but I feel the following operation could be factorized in a loop:

await expect(workflowVisualizer.getAllStepNodes()).toContainText(ACTIONS); await expect(workflowVisualizer.getAllStepNodes()).toHaveCount(COUNT); await workflowVisualizer.deleteStep(CURR_STEP);

In my opinion would make it easier to read and allow a better extensibility in case we create a new workflow actions in the future ? ( If the test has for goal to always create all of them in once workflow, if it's even possible )

I thought about doing a loop, but I'm unsure if it would make the test's code more transparent. In the test, I'm alternatively deleting the first and last nodes of the flow. I prefer dumb code instead of writing complex code to reach the same goal.

I'm open to trying different patterns in e2e tests so we find the best one for the team.

prastoin · 2025-01-20T15:38:20Z

packages/twenty-e2e-testing/lib/fixtures/blank-workflow.ts

 export class WorkflowVisualizerPage {
  #page: Page;

  workflowId: string;
  workflowName: string;

+  readonly addStepButton: Locator;


Praise love all of it

thomtrp

Those are great 🔥 Do you keep deactivation for later?

thomtrp · 2025-01-21T10:13:07Z

packages/twenty-e2e-testing/lib/fixtures/blank-workflow.ts

+  | 'update-record'
+  | 'delete-record'
+  | 'code'
+  | 'send-email';


maybe types should go in a separated file. I feel like you will need it in other tests

Done. Thank you!

thomtrp · 2025-01-21T10:17:32Z

packages/twenty-e2e-testing/lib/fixtures/blank-workflow.ts

+    });
+    this.deactivateWorkflowButton = page.getByLabel('Deactivate Workflow', {
+      exact: true,
+    });


why do we have deactivate and activate?

What do you mean by this question?

This code sets up some "locators". We should think about Playwright's locators as CSS selectors. A string containing a CSS selector won't be resolved until we provide it to document.querySelector().
Here, I declare how we can find the activate and deactivate buttons. However, these locators will only be resolved when I use them with expect(locator) or locator.click().

github-actions · 2025-01-21T10:47:17Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    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-8c32a146.json

Generated by 🚫 dangerJS against 281640b

greptile-apps bot reviewed Jan 20, 2025

View reviewed changes

prastoin reviewed Jan 20, 2025

View reviewed changes

Devessier added -PR: awaiting author run-e2e labels Jan 20, 2025

Devessier force-pushed the workflow-e2e-batch-2 branch from 77a5a52 to c5a9fc9 Compare January 20, 2025 17:17

Devessier added -PR: awaiting review and removed -PR: awaiting author labels Jan 20, 2025

Devessier added 8 commits January 21, 2025 11:04

test: update test and add a test to create every possible action

ff288fe

test: migrate test to POM

d2100dc

test: delete redundant test

2f82c1a

test: create a reusable function to create a trigger

a2d8b0c

test: ensure steps can be deleted from a draft version

655f6cf

test: assert we can add a step to an active version

a4a9291

test: assert we can replace the trigger of an active version

625ee39

test: update the expected name of actions

df4a11a

thomtrp reviewed Jan 21, 2025

View reviewed changes

refactor: extract types in another file

281640b

Devessier force-pushed the workflow-e2e-batch-2 branch from c5a9fc9 to 281640b Compare January 21, 2025 10:33

thomtrp approved these changes Jan 21, 2025

View reviewed changes

Devessier merged commit 8e0467e into main Jan 21, 2025
47 checks passed

Devessier deleted the workflow-e2e-batch-2 branch January 21, 2025 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workflow E2E tests – batch 2 #9747

Workflow E2E tests – batch 2 #9747

Devessier commented Jan 20, 2025 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Jan 20, 2025

greptile-apps bot Jan 20, 2025

greptile-apps bot Jan 20, 2025

prastoin left a comment

prastoin Jan 20, 2025

Devessier Jan 20, 2025

prastoin Jan 20, 2025

thomtrp left a comment •

edited

Loading

thomtrp Jan 21, 2025

Devessier Jan 21, 2025

thomtrp Jan 21, 2025

Devessier Jan 21, 2025

github-actions bot commented Jan 21, 2025


		await workflowVisualizer.deleteStep(fifthStepId);

		await expect(workflowVisualizer.getAllStepNodes()).toContainText([

Workflow E2E tests – batch 2 #9747

Workflow E2E tests – batch 2 #9747

Conversation

Devessier commented Jan 20, 2025 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Jan 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 20, 2025

Choose a reason for hiding this comment

prastoin left a comment

Choose a reason for hiding this comment

prastoin Jan 20, 2025

Choose a reason for hiding this comment

Devessier Jan 20, 2025

Choose a reason for hiding this comment

prastoin Jan 20, 2025

Choose a reason for hiding this comment

thomtrp left a comment • edited Loading

Choose a reason for hiding this comment

thomtrp Jan 21, 2025

Choose a reason for hiding this comment

Devessier Jan 21, 2025

Choose a reason for hiding this comment

thomtrp Jan 21, 2025

Choose a reason for hiding this comment

Devessier Jan 21, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 21, 2025

Log

Devessier commented Jan 20, 2025 •

edited

Loading

thomtrp left a comment •

edited

Loading