Skip to content

Conversation

@JakeQZ
Copy link
Collaborator

@JakeQZ JakeQZ commented Dec 31, 2025

This method will make it easier to test the parsing independently of the rendering.

Placeholder implementations are currently all stubs that will throw an exception, confirmed by tests.
They can be implemented as and when required, with the tests updated.

Part of #1440.

This method will make it easier to test the parsing independently of the
rendering.

Placeholder implementations are currently all stubs that will throw an
exception, confirmed by tests.
They can be implemented as and when required, with the tests updated.

Part of #1440.
@JakeQZ JakeQZ requested a review from oliverklee December 31, 2025 00:37
@JakeQZ JakeQZ self-assigned this Dec 31, 2025
@JakeQZ JakeQZ added the testing PRs/issues adding additional tests only, or primarily testing-focused label Dec 31, 2025
@coveralls
Copy link

Coverage Status

coverage: 69.507% (+0.3%) from 69.191%
when pulling 28c3f16 on test/helper/getarrayrepresentation
into 0ae1fde on main.

@oliverklee oliverklee added developer-specific Issues that only affect maintainers, contributors, and people submitting PRs enhancement labels Dec 31, 2025
Copy link
Collaborator

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

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

We also should have the stub implementation (plus tests) in the concrete subclasses of the (usually) abstract classes as the real implementations usually will be different for each class:

  • CSSList:
    • KeyFrame
    • CSSBlockList:
      • AtRuleBlockList
      • Document
    • AtRuleBlockList
  • ValueList:
    • CSSFunction
    • CalcFunction
    • Color
    • LineName
    • RuleValueList
    • CalcRuleValueList
  • PrimitiveValue:
    • CSSString
    • URL
    • Size
  • Selector:
    • KeyframeSelector

(I hope I haven't missed any class.)

@oliverklee
Copy link
Collaborator

I'd also be fine with doing that in a follow-up PR.

Copy link
Collaborator

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

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

Ah, well, let's merge this and add the methods to the subclasses in a separate PR:

@oliverklee oliverklee merged commit b74a000 into main Dec 31, 2025
23 checks passed
@oliverklee oliverklee deleted the test/helper/getarrayrepresentation branch December 31, 2025 12:31
@JakeQZ
Copy link
Collaborator Author

JakeQZ commented Dec 31, 2025

We also should have the stub implementation (plus tests) in the concrete subclasses of the (usually) abstract classes as the real implementations usually will be different for each class:

I'm not sure that's necessary. We can just add the real implementations as and when they are needed. If a child class ends up inheriting a parent implementation, but should have a different implementation (probably extended with additional properties), it will be obvious when we come to use it in testing that the child class method needs to be implemented (and tested) first, since it will not return (all) the properties we want to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer-specific Issues that only affect maintainers, contributors, and people submitting PRs enhancement testing PRs/issues adding additional tests only, or primarily testing-focused

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants