Skip to content

[TASK] Add tests for the selector specificity #1025

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

Merged
merged 2 commits into from
Mar 1, 2025
Merged

Conversation

oliverklee
Copy link
Collaborator

@oliverklee oliverklee commented Feb 27, 2025

Mostly move tests from ParserTest that belong into
the unit tests.

Part of #757
Part of #758

@coveralls
Copy link

coveralls commented Feb 27, 2025

Coverage Status

coverage: 55.753% (+0.5%) from 55.282%
when pulling efe6034 on task/tests/selector-1
into 19ffd07 on main.

@oliverklee oliverklee force-pushed the task/tests/selector-1 branch 3 times, most recently from 2572d9a to bce6f45 Compare February 27, 2025 21:02
Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

I think there's a naming issue. MDN does indeed say:

In CSS, selectors are patterns used to match, or select, the elements you want to style...

"pattern" is a description of how they work. "selector" is the (official) name for what they are. I think the test names should reflect what the 'object' is called (or known as), rather than what it does or how it works.

Wittgenstein might disagree (if I sit on a table, it's now a chair), but he wasn't a computer scientist.

Also, I'd like this PR to be split for more manageable review - the addition of the deprecated test class can be done separately I think.

@oliverklee
Copy link
Collaborator Author

I'd like to find a way around that a selector can have a selector (which would mean that we need to deal with a selector's selector) … any ideas?

@JakeQZ
Copy link
Collaborator

JakeQZ commented Feb 28, 2025

I'd like to find a way around that a selector can have a selector (which would mean that we need to deal with a selector's selector) … any ideas?

The Selector class represents and/or encapsulates a selector, which in the rendered or to-be-parsed CSS is just a string like a:hover. I guess the string property of the class could be named something like $rawContent or $stringRepresentation, or something better that I can't come up with right now. Note we alse have Value.value, and others (e.g. Rule.rule).

@oliverklee oliverklee force-pushed the task/tests/selector-1 branch from bce6f45 to 80fd492 Compare February 28, 2025 08:04
@oliverklee oliverklee marked this pull request as draft February 28, 2025 08:14
@oliverklee
Copy link
Collaborator Author

Okay, then I'll keep using selector for the tests for now, and we do the renaming in #1016. I'll also split this up into smaller chunks.

@oliverklee oliverklee force-pushed the task/tests/selector-1 branch from 80fd492 to 7365d9b Compare February 28, 2025 19:58
Mostly move tests from `ParserTest` that belong into
the unit tests.

Part of #757
Part of #758
@oliverklee oliverklee changed the title [TASK] Add the first tests for Selector [TASK] Add tests for the selector specificity Feb 28, 2025
@oliverklee oliverklee force-pushed the task/tests/selector-1 branch from 7365d9b to b271be2 Compare February 28, 2025 20:03
@oliverklee oliverklee marked this pull request as ready for review February 28, 2025 20:04
@oliverklee oliverklee requested a review from JakeQZ February 28, 2025 20:04
@oliverklee
Copy link
Collaborator Author

The other two PRs are merged so that now this PR is a lot smaller. Rebased, reworded and repushed.

Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

There's a typo in a test method name.

Specificites cannot be represented by a single number, so we should address that in future...

@oliverklee oliverklee requested a review from JakeQZ March 1, 2025 00:24
@JakeQZ
Copy link
Collaborator

JakeQZ commented Mar 1, 2025

Specificites cannot be represented by a single number, so we should address that in future...

Added #1042 to capture this.

@JakeQZ JakeQZ merged commit 7ef82db into main Mar 1, 2025
21 checks passed
@JakeQZ JakeQZ deleted the task/tests/selector-1 branch March 1, 2025 00:38
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