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

EColorPicker.spec.js: delete "accepts 3-digit hex color codes, after picker has been shown" #5370

Merged
merged 1 commit into from
Jun 16, 2024

Conversation

BacLuc
Copy link
Contributor

@BacLuc BacLuc commented Jun 16, 2024

Because it is flaky now.

You can see here if the frontend tests are stable now: https://github.com/BacLuc/ecamp3/actions/runs/9534905924/job/26279962021

Copy link
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

I don't like skipping tests unless we have a clear cutoff date (e.g. the release of a fix in a third-party package) when we know we can re-activate the test. Without a strong reason to re-activate it, nobody is going to. I'd prefer deleting the test in that case, because it will be unmaintained and it will feel risky to re-activate a flaky test.
Another way to see it: If you were writing new tests and you couldn't get one of them to run without being flaky, you would just omit it instead of adding a skipped test to the test suite.

…picker has been shown"

Because it is flaky now.
@BacLuc BacLuc changed the title EColorPicker.spec.js: skip "accepts 3-digit hex color codes, after picker has been shown" EColorPicker.spec.js: delete "accepts 3-digit hex color codes, after picker has been shown" Jun 16, 2024
@BacLuc
Copy link
Contributor Author

BacLuc commented Jun 16, 2024

I don't like skipping tests unless we have a clear cutoff date (e.g. the release of a fix in a third-party package) when we know we can re-activate the test. Without a strong reason to re-activate it, nobody is going to. I'd prefer deleting the test in that case, because it will be unmaintained and it will feel risky to re-activate a flaky test. Another way to see it: If you were writing new tests and you couldn't get one of them to run without being flaky, you would just omit it instead of adding a skipped test to the test suite.

done

We need to move forward, we cannot merge anything now.

Copy link
Member

@manuelmeister manuelmeister left a comment

Choose a reason for hiding this comment

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

Thanks! Sadly we can't stable test this with jsdom, as it can't use getComputedStyle. I'll add this test as a cypress component test.

@manuelmeister manuelmeister added this pull request to the merge queue Jun 16, 2024
Merged via the queue into ecamp:devel with commit a72e7da Jun 16, 2024
32 checks passed
@BacLuc BacLuc deleted the skip-frontend-test branch June 16, 2024 11:15
@BacLuc BacLuc mentioned this pull request Jul 10, 2024
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.

3 participants