-
Notifications
You must be signed in to change notification settings - Fork 98
Enable Boolean op deletion from the feature tree #6637
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
Enable Boolean op deletion from the feature tree #6637
Conversation
Fixes #6584. Works locally but the new tests don't appear to work yet
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| selection.artifact && | ||
| selection.artifact.type !== 'sweep' && | ||
| selection.artifact.type !== 'plane' && | ||
| selection.artifact.type !== 'compositeSolid' && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Irev-Dev I just added to the existing codepath here for point and click var declarations 🤷
| await test.step(`Delete ${operationName} operation via feature tree selection`, async () => { | ||
| await toolbar.openPane('feature-tree') | ||
| const ftOp = await toolbar.getFeatureTreeOperation(operationName, 0) | ||
| await ftOp.click({ button: 'left' }) | ||
| await page.keyboard.press('Delete') | ||
| await scene.settled(cmdBar) | ||
| await toolbar.closePane('feature-tree') | ||
|
|
||
| // Expect changes in ft and code | ||
| await toolbar.openPane('code') | ||
| await editor.expectEditor.not.toContain(operation.code) | ||
| await expect( | ||
| await toolbar.getFeatureTreeOperation(operationName, 0) | ||
| ).not.toBeVisible() | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow this wasn't working well locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which line was failing? I'm guessing this one?
await editor.expectEditor.not.toContain(operation.code)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just doesn't delete in the test, so weird. Working on it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with right click -> Delete and that seems much better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is it possible that execution and population of the feature tree were taking longer than the test could allow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also added scene.settled, should be better I think!
franknoirot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirmed using the sample code from #6584 on a local build that this fixes the issue. Thanks!
Fixes #6584.
Works locally but the new tests don't appear to work yet