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

Enable tests #1389

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented Dec 7, 2023

Enabled some disabled tests which aren't in broken state anymore and fixed other disabled tests.
eclipse-platform/eclipse.platform#525

In the MediaRulesTest class, we test for if a CSS rule with @media tag is considered on parsing. Ideally we want the rule to be ignored.
Suppose we have a CSS string like:
@media screen, print {
BODY { background-color: red }
}
In the previous version of the test (which was disabled), it was expected that the rule "BODY { background-color: red }" under @media tag would be completely ignored. But now the behaviour has changed. It seems like the rules under @media are loaded indeed on parsing but the attributes are ignored, i.e. it looks like "BODY { }".

However, this behaviour doesn't have any side effect since a rule with no attributes neither change anything nor resets anything.

@amartya4256 amartya4256 force-pushed the enable_tests branch 4 times, most recently from 3f28b7a to 85631cd Compare December 8, 2023 08:27
@fedejeanne
Copy link
Contributor

The failure of org.eclipse.ui.tests.progress.ProgressViewTests.testItemOrder might be unrelated to your changes (and it probably is). Please look for any existing issue on that (and, if there is none, please create one) and mention it here.

Copy link
Contributor

github-actions bot commented Dec 8, 2023

Test Results

   903 files  ±0     903 suites  ±0   45m 19s ⏱️ - 2m 21s
 7 474 tests ±0   7 322 ✅ +1  151 💤  - 2  1 ❌ +1 
23 571 runs  ±0  23 063 ✅ +3  505 💤  - 6  3 ❌ +3 

For more details on these failures, see this check.

Results for commit e4d26d6. ± Comparison against base commit c45ca46.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

Better. You could add a use case that checks that the order doesn't play a role and extract a private method to improve readability

@amartya4256 amartya4256 force-pushed the enable_tests branch 2 times, most recently from 43a69c6 to 2b3badb Compare December 11, 2023 12:37
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

testSelectPseudo is failing. Please investigate and fix or undo changes in it

amartya4256 and others added 3 commits January 19, 2024 14:24
…ests/css/core/parser/MediaRulesTest.java

Co-authored-by: Federico Jeanne <2205684+fedejeanne@users.noreply.github.com>
…sts/css/swt/CTabItemTest.java


Don't disable test
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