Add .github/workflows/e2e.yml #2968

aidavoxel51 · 2023-05-05T20:01:07Z

What changes are proposed in this pull request?

Added e2e.yml to run e2e/* tests in pull requests to develop.

How is this patch tested? If it is not, please explain why.

Test job run https://github.com/voxel51/fiftyone/actions/runs/4887646675/jobs/8724476236

Release Notes

Is this a user-facing change that should be mentioned in the release notes?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release
notes for FiftyOne users.

(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)

What areas of FiftyOne does this PR affect?

App: FiftyOne application changes
Build: Build and test infrastructure changes
Core: Core fiftyone Python library changes
Documentation: FiftyOne documentation changes
Other

…orkflows/e2e.yml

codecov · 2023-05-05T20:10:29Z

Codecov Report

Patch coverage: 73.68% and project coverage change: -13.57 ⚠️

Comparison is base (1fa069e) 62.18% compared to head (39d204c) 48.61%.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #2968       +/-   ##
============================================
- Coverage    62.18%   48.61%   -13.57%     
============================================
  Files          250      227       -23     
  Lines        45507    33359    -12148     
  Branches       319      319               
============================================
- Hits         28297    16217    -12080     
+ Misses       17210    17142       -68

Flag	Coverage Δ
app	`48.61% <73.68%> (+0.09%)`	⬆️
python	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ages/components/src/components/Results/Results.tsx	`29.80% <0.00%> (-1.20%)`	⬇️
app/packages/state/src/recoil/sidebar.ts	`50.53% <100.00%> (+0.46%)`	⬆️

... and 29 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

sashankaryal · 2023-05-08T16:13:20Z

e2e/index.ts

+  // if (!options.headed) {
+  //   throw new Error(
+  //     "Cypress is running in headless mode, but screenshotting doesn't work as expected in this mode with looker. See https://github.com/cypress-io/cypress/issues/15605 for more details."
+  //   );
+  // }


could you uncomment these?

@benjaminpkane @aidavoxel51 I couldn't get headless mode to work with screenshotting canvases. If it works, then great!!

Yeah, headed is required. Reverted

benjaminpkane

Looks good! A couple small comments. I am looking into fixing the tests themselves right now

benjaminpkane · 2023-05-09T14:04:48Z

.github/workflows/e2e.yml

@@ -0,0 +1,80 @@
+name: Test e2e
+
+on:


Following the existing pattern, can we have this workflow run via on: workflow_call?

The idea would be we add the workflow call to the PR and Publish workflows.

https://github.com/voxel51/fiftyone/blob/develop/.github/workflows/pr.yml

https://github.com/voxel51/fiftyone/blob/develop/.github/workflows/publish.yml

And add it to the needs lists so the workflows fail/reject when e2e tests fail

fiftyone/.github/workflows/pr.yml

Line 18 in 327789b

needs: [build, test]

fiftyone/.github/workflows/publish.yml

Line 19 in 327789b

needs: [build, test]

fiftyone/.github/workflows/publish.yml

Line 43 in 327789b

needs: [build, test]

benjaminpkane · 2023-05-09T14:09:09Z

.github/workflows/e2e.yml

+      - name: FFmpeg
+        uses: FedericoCarboni/setup-ffmpeg@v2
+
+      - name: Run e2e tests


The test step is currently passing when tests fail. Ideally this will fail when tests fail so the workflow calls can then fail

I saw you made the changes, were you able to test it?

Yes, it works. The process will exit nonzero if 1 or more tests fail

sashankaryal · 2023-05-09T16:27:46Z

.github/workflows/e2e.yml

+
+      - name: Run e2e tests
+        run: |
+          FIFTYONE_DATABASE_NAME=cypress python ../fiftyone/server/main.py --address 0.0.0.0 --port 8787 &


@benjaminpkane Is this supposed to replace the fake server? 🥳 that's cool, but yarn start will no longer start cypress correctly locally, no?

I guess we could add something like:

"cy:run": "FIFTYONE_DATABASE_NAME=cypress python ../fiftyone/server/main.py --address 0.0.0.0 --port 8787 & cypress run"

Yes, I'll tweak it

I'm proposing the background server is started manually by the developer when testing locally with yarn start-test-server. I updated the readme. Trying to avoid creating orphaned processes

benjaminpkane

LGTM. I'm comfortable merging. I did end up removing some of the existing cypress subprocess code in favor of a blocking approach to the Python subprocess execution. @sashankaryal @aidavoxel51

* Add .github/workflows/e2e.yml * Update yml name .github/workflows/e2e.yml * Update from workflow_dispatch to on:push:branches:e2e-ci in .github/workflows/e2e.yml * Remove yarn build .github/workflows/e2e.yml * Add env ELECTRON_EXTRA_LAUNCH_ARGS to e2e.yml * Add .github/workflows/e2e.yml * Update yml name .github/workflows/e2e.yml * Update from workflow_dispatch to on:push:branches:e2e-ci in .github/workflows/e2e.yml * Remove yarn build .github/workflows/e2e.yml * Add env ELECTRON_EXTRA_LAUNCH_ARGS to e2e.yml * use 0.0.0.0 * remove wait-on * use 0.0.0.0 in ghost server as well * improve test resilience * increase ghost server timeout * increase app wait timeout * increase app wait timeout even more :( * run without cypress actions workflow * correct syntax-error on working-directory * use ubuntu latest * use ubuntu latest * Add npm install --save-dev tsconfig-paths * Add yarn install =) * Change DEFAULT_APP_HOSTNAME in e2e/lib/constants.ts * Change DEFAULT_APP_HOSTNAME in e2e * Add debug logs * Switch --headful to --headless in e2e/index.ts * Comment out options.headed{} * Add fiftyone app connect command * Fix line * Add debugging * Correct connect * Add set -x * Set remote to false * Update DEFAULT_APP_ADDRESS * Add debugging * make local tests pass * use event driven way to shut ghost server * background server * update name * testing * testing * cypress * experiment * mv startup * tweaks * path fix * no wait * typo * wait * upload artifacts * node change * edit * Add build steps * Add step Install fiftyone in e2e.yml * 0.0.0.0 * Add trigger and cleanup * short circuit * fix modal * headed * cleanup * update workflows * edits * Add e2e to pr.yml and publish.yml * edits * exit code * update e2e readme --------- Co-authored-by: Sashank Aryal <[email protected]> Co-authored-by: Benjamin Kane <[email protected]>

aidavoxel51 and others added 30 commits March 31, 2023 16:23

Add .github/workflows/e2e.yml

66d0a26

Update yml name .github/workflows/e2e.yml

e4776b0

Update from workflow_dispatch to on:push:branches:e2e-ci in .github/w…

81990d6

…orkflows/e2e.yml

Remove yarn build .github/workflows/e2e.yml

60f3b2b

Add env ELECTRON_EXTRA_LAUNCH_ARGS to e2e.yml

48373d9

Merge branch 'develop' into e2e-ci

1c67627

Add .github/workflows/e2e.yml

a3ad5de

Update yml name .github/workflows/e2e.yml

366b02e

Update from workflow_dispatch to on:push:branches:e2e-ci in .github/w…

f30e8d3

…orkflows/e2e.yml

Remove yarn build .github/workflows/e2e.yml

b542682

Add env ELECTRON_EXTRA_LAUNCH_ARGS to e2e.yml

2854750

use 0.0.0.0

e0ad8a4

remove wait-on

56a2e66

use 0.0.0.0 in ghost server as well

7745beb

improve test resilience

7fd2fa3

increase ghost server timeout

1a150cf

increase app wait timeout

1303d54

increase app wait timeout even more :(

b7faf49

run without cypress actions workflow

538efac

correct syntax-error on working-directory

b1a4089

use ubuntu latest

bdf8a95

use ubuntu latest

61f286c

Add npm install ts-node

62ed040

Add npm install --save-dev tsconfig-paths

d97efb3

Add yarn install =)

140c679

Change DEFAULT_APP_HOSTNAME in e2e/lib/constants.ts

e40a1b5

Change DEFAULT_APP_HOSTNAME in e2e

7149ab3

Add debug logs

8f31b17

Switch --headful to --headless in e2e/index.ts

6482833

Comment out options.headed{}

1769a5e

aidavoxel51 and others added 3 commits May 4, 2023 12:35

Add step Install fiftyone in e2e.yml

7a56d6d

0.0.0.0

e01ba55

Add trigger and cleanup

fb399d5

aidavoxel51 requested review from benjaminpkane, sashankaryal and a team May 5, 2023 20:01

aidavoxel51 self-assigned this May 5, 2023

sashankaryal reviewed May 8, 2023

View reviewed changes

benjaminpkane requested changes May 9, 2023

View reviewed changes

Merge branch 'develop' into e2e-ci

0613898

sashankaryal reviewed May 9, 2023

View reviewed changes

benjaminpkane added 7 commits May 9, 2023 10:37

short circuit

607b93c

Merge branch 'develop' into e2e-ci

1ee185f

fix modal

a084801

Merge branch 'fix-modal' into e2e-ci

68c601a

headed

95b7bb6

cleanup

f48c963

update workflows

2206626

benjaminpkane force-pushed the e2e-ci branch from 39b12fc to 2206626 Compare May 9, 2023 19:52

benjaminpkane and others added 6 commits May 9, 2023 14:37

edits

36b816a

Add e2e to pr.yml and publish.yml

1fd3e2a

Updated branch and resolved conflicts

105155c

edits

6b12ca4

exit code

85b23f3

update e2e readme

39d204c

benjaminpkane approved these changes May 11, 2023

View reviewed changes

benjaminpkane merged commit c9f37ad into develop May 15, 2023

benjaminpkane deleted the e2e-ci branch May 15, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add .github/workflows/e2e.yml #2968

Add .github/workflows/e2e.yml #2968

aidavoxel51 commented May 5, 2023

codecov bot commented May 5, 2023 •

edited

Loading

sashankaryal May 8, 2023

sashankaryal May 9, 2023

benjaminpkane May 9, 2023

benjaminpkane left a comment

benjaminpkane May 9, 2023

benjaminpkane May 9, 2023

aidavoxel51 May 11, 2023

benjaminpkane May 11, 2023

sashankaryal May 9, 2023

sashankaryal May 9, 2023

benjaminpkane May 9, 2023

benjaminpkane May 11, 2023

benjaminpkane left a comment •

edited

Loading

Add .github/workflows/e2e.yml #2968

Add .github/workflows/e2e.yml #2968

Conversation

aidavoxel51 commented May 5, 2023

What changes are proposed in this pull request?

How is this patch tested? If it is not, please explain why.

Release Notes

Is this a user-facing change that should be mentioned in the release notes?

What areas of FiftyOne does this PR affect?

codecov bot commented May 5, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjaminpkane left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjaminpkane left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented May 5, 2023 •

edited

Loading

benjaminpkane left a comment •

edited

Loading