Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Hating on HTML fixtures #1306

Merged
merged 4 commits into from
Feb 9, 2014
Merged

Hating on HTML fixtures #1306

merged 4 commits into from
Feb 9, 2014

Conversation

xaviershay
Copy link
Member

The HTML fixtures churn a lot, are a PITA to re-generate, and don't provide a lot of value. Here is an alternate suggestion:

  • Store mildly processed HTML that strips out stuff we don't care about.
  • Only verify the fixture on modern rubies that all generate the same HTML.
  • Older rubies still run a smoke test ensuring the HTML generation does not explode. If they succeed at this, it is highly unlikely that their output will be significantly different from a modern ruby that we are validating the fixture on.

@myronmarston @JonRowe @soulcutter @samphippen

This means less churn in diffs. We weren't asserting on these lines
anyway.
This still gives us a high level of confidence in the HTML formatter,
while drastically reducing the complexity of the specs. We only require
a single fixture with this method.
@JonRowe
Copy link
Member

JonRowe commented Feb 9, 2014

Seems legit to me, although you're right the fixture script won't work at all for me ;)

@xaviershay
Copy link
Member Author

I was torn ... everyone is going to have a different script, but it seems better to have something here rather than nothing? Though if we merge this basically as is the need for it pretty much goes away.

@myronmarston
Copy link
Member

This has never been a pain point for me, but these are definite improvements, so LGTM. Merge away.

@xaviershay
Copy link
Member Author

I currently can't compile 1.9.2 so can't get any of my PRs green. Too much yak shaving for a Sunday. This will be a big help.

xaviershay added a commit that referenced this pull request Feb 9, 2014
@xaviershay xaviershay merged commit 940ffa1 into master Feb 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants