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

Add traces to integration tests + suppress one flaky assertion. #11595

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

farmaazon
Copy link
Contributor

@farmaazon farmaazon commented Nov 19, 2024

Pull Request Description

Fixes #11604

Most issues were caused by a problem with Project List flooding the network with its requests - this was fixed on develop.
But one assertion was flaky - it assumed we will see the "real" run result on write node, but sometimes it is immediately overwritten by dry run.

But the most important part of this PR is adding traces to Electron packages - it's should be much easier now to debug E2E test failures.

Also renamed the previously misnamed "E2E tests" to "[GUI] integration tests".

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • [ ] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • [ ] Unit tests have been written where possible.
  • [ ] If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@farmaazon farmaazon added the CI: No changelog needed Do not require a changelog entry for this PR. label Nov 21, 2024
@farmaazon farmaazon force-pushed the wip/farmaazon/fix-integration-test branch from ff787de to 43bbad4 Compare November 21, 2024 13:19
@farmaazon farmaazon changed the base branch from develop to wip/sb/fix-react-compiler-lints November 21, 2024 13:21
Base automatically changed from wip/sb/fix-react-compiler-lints to develop November 21, 2024 13:49
@farmaazon farmaazon force-pushed the wip/farmaazon/fix-integration-test branch from b8089aa to b91e7c4 Compare November 21, 2024 13:54
@farmaazon farmaazon force-pushed the wip/farmaazon/fix-integration-test branch from b91e7c4 to c078f4d Compare November 21, 2024 14:25
Copy link

github-actions bot commented Nov 21, 2024

🧪 Storybook is successfully deployed!

📊 Dashboard:

@farmaazon farmaazon changed the title Add traces to integration tests Add traces to integration tests + suppress one flacky assertion. Nov 22, 2024
@farmaazon farmaazon changed the title Add traces to integration tests + suppress one flacky assertion. Add traces to integration tests + suppress one flaky assertion. Nov 22, 2024
@farmaazon farmaazon self-assigned this Nov 22, 2024
@farmaazon farmaazon marked this pull request as ready for review November 22, 2024 13:50
await expect(writeNode.locator('.TableVisualization')).toContainText(OUTPUT_FILE)
} catch {
// TODO[ao]
// The above check is flaky, because sometimes the additional egine run overrides node output back to "dry run".
Copy link
Contributor

Choose a reason for hiding this comment

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

egine -> engine

Copy link
Contributor

@somebody1234 somebody1234 left a comment

Choose a reason for hiding this comment

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

approving rename of dashboard files only

@somebody1234
Copy link
Contributor

might wanna update codeowners too due to the change of path

@MrFlashAccount
Copy link
Contributor

@farmaazon why we run pw tests in unit tests job?
CleanShot 2024-11-22 at 17 50 14@2x

Copy link
Contributor

@MrFlashAccount MrFlashAccount left a comment

Choose a reason for hiding this comment

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

See the comment above

@farmaazon farmaazon force-pushed the wip/farmaazon/fix-integration-test branch from 697bf06 to 7092738 Compare November 22, 2024 15:18
@mergify mergify bot mentioned this pull request Nov 23, 2024
5 tasks
@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E2E tests on electron package have multiple false failures.
3 participants