Skip to content

[TASK] Add getArrayRepresentation() to Renderable - #1447

Merged
oliverklee merged 1 commit into
mainfrom
test/helper/getarrayrepresentation
Dec 31, 2025
Merged

[TASK] Add getArrayRepresentation() to Renderable#1447
oliverklee merged 1 commit into
mainfrom
test/helper/getarrayrepresentation

Conversation

@JakeQZ

@JakeQZ JakeQZ commented Dec 31, 2025

Copy link
Copy Markdown
Collaborator

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
Copy Markdown

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

@oliverklee oliverklee left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator

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

@oliverklee oliverklee left a comment

Copy link
Copy Markdown
Collaborator

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

JakeQZ commented Dec 31, 2025

Copy link
Copy Markdown
Collaborator Author

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.

3 participants