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.

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

Tree item abort #6757

Merged
merged 18 commits into from Jul 14, 2023
Merged

Tree item abort #6757

merged 18 commits into from Jul 14, 2023

Conversation

jvigliotta
Copy link
Contributor

Closes #6753

Describe your changes:

Grabbing the abort signal in the tree and passing it to composition.load so requests can be canceled if item closed.
Suppressing all console.error's for aborted requests in couch. This information is available in the network tab.
Added an e2e regression test for this feature.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

  • Changes appear to address issue?
  • Reviewer has tested changes by following the provided instructions?
  • Changes appear not to be breaking changes?
  • Appropriate automated tests included?
  • Code style and in-line documentation are appropriate?
  • Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
  • Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

@jvigliotta jvigliotta requested a review from ozyx June 23, 2023 19:56
@deploysentinel
Copy link

deploysentinel bot commented Jun 23, 2023

Current Playwright Test Results Summary

✅ 14 Passing

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/14/2023 05:48:56pm UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 7a389fa

Started: 07/14/2023 05:46:03pm UTC

View Detailed Build Results


Current Playwright Test Results Summary

✅ 133 Passing - ⚠️ 4 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/14/2023 05:48:56pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 7a389fa

Started: 07/14/2023 05:42:41pm UTC

⚠️ Flakes

📄   functional/plugins/conditionSet/conditionSet.e2e.spec.js • 3 Flakes

Top 1 Common Error Messages

null

3 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Condition Set CRUD Operations on @localStorage Condition set object properties persist in main view and inspector @localStorage
Retry 1Initial Attempt
-0.70% (-1) -1 / 142 runs
failed over last 7 days
8.45% (12) 12 / 142 runs
flaked over last 7 days
Condition Set CRUD Operations on @localStorage condition set object can be modified on @localStorage
Retry 1Initial Attempt
-9.16% (-12) -12 / 131 runs
failed over last 7 days
9.16% (12) 12 / 131 runs
flaked over last 7 days
Condition Set CRUD Operations on @localStorage condition set object can be deleted by Search Tree Actions menu on @localStorage
Retry 1Initial Attempt
-8.40% (-11) -11 / 131 runs
failed over last 7 days
9.16% (12) 12 / 131 runs
flaked over last 7 days

📄   functional/plugins/imagery/exampleImagery.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Example Imagery Object Can use Mouse Wheel to zoom in and out of latest image
Retry 1Initial Attempt
0.65% (1) 1 / 155 run
failed over last 7 days
80% (124) 124 / 155 runs
flaked over last 7 days

View Detailed Build Results


@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #6757 (3b55358) into master (cde8fbb) will increase coverage by 0.86%.
The diff coverage is 40.00%.

❗ Current head 3b55358 differs from pull request most recent head 7a389fa. Consider uploading reports for the commit 7a389fa to get more accurate results

@@            Coverage Diff             @@
##           master    #6757      +/-   ##
==========================================
+ Coverage   52.92%   53.78%   +0.86%     
==========================================
  Files         628      628              
  Lines       25021    25027       +6     
  Branches     2505     2508       +3     
==========================================
+ Hits        13242    13462     +220     
+ Misses      11106    10893     -213     
+ Partials      673      672       -1     
Flag Coverage Δ *Carryforward flag
e2e-full 42.17% <ø> (+0.01%) ⬆️ Carriedforward from 0297a0d
e2e-stable 55.68% <0.00%> (+5.74%) ⬆️
unit 48.31% <40.00%> (+0.06%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
src/ui/components/viewControl.vue 0.00% <0.00%> (ø)
src/ui/layout/mct-tree.vue 9.78% <0.00%> (ø)
src/ui/layout/tree-item.vue 0.00% <ø> (ø)
...c/plugins/persistence/couch/CouchObjectProvider.js 91.02% <50.00%> (-0.57%) ⬇️
src/api/objects/ObjectAPI.js 93.08% <75.00%> (-0.44%) ⬇️

... and 44 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cde8fbb...7a389fa. Read the comment docs.

Copy link
Member

@ozyx ozyx left a comment

Choose a reason for hiding this comment

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

Looks great overall, just had one question for you regarding AbortError handling in the CouchObjectProvider

Copy link
Member

@ozyx ozyx left a comment

Choose a reason for hiding this comment

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

One small nitpick that you can take or leave, but LGTM otherwise!

src/ui/components/viewControl.vue Show resolved Hide resolved
Copy link
Member

@ozyx ozyx left a comment

Choose a reason for hiding this comment

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

LGTM, a little confused about the previous test failure though. False positive?

src/api/objects/ObjectAPI.js Show resolved Hide resolved
src/api/objects/ObjectAPISpec.js Show resolved Hide resolved
@jvigliotta jvigliotta requested a review from akhenry July 11, 2023 17:16
Copy link
Contributor

@akhenry akhenry left a comment

Choose a reason for hiding this comment

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

Looks good to me, ship it.

@akhenry akhenry enabled auto-merge (squash) July 13, 2023 01:51
@akhenry akhenry added the pr:e2e:couchdb npm run test:e2e:couchdb label Jul 13, 2023
@github-actions github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Jul 13, 2023
@unlikelyzero unlikelyzero added the pr:e2e:couchdb npm run test:e2e:couchdb label Jul 13, 2023
@github-actions github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Jul 13, 2023
@unlikelyzero
Copy link
Collaborator

This is blocked on a real bug on the couchdb tests

@jvigliotta jvigliotta added the pr:e2e:couchdb npm run test:e2e:couchdb label Jul 14, 2023
@github-actions github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Jul 14, 2023
@akhenry akhenry merged commit 92329b3 into master Jul 14, 2023
12 of 13 checks passed
@akhenry akhenry deleted the tree-item-abort branch July 14, 2023 17:49
@ozyx ozyx added this to the Target:3.0.0 milestone Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MCTTree] We've lost abort on closing of folders, could lead to bad performance
4 participants