search

rprt / rspec-page-regression

Rspec-page-regression provides a mechanism for regression testing of web page renders in RSpec. It takes into account HTML, CSS, and JavaScript, by virtue of using PhantomJS (via the Poltergeist gem) or Selenium to render snapshots. It provides an RSpec matcher that compares the test snapshot to an expected image, and facilitates management of the images.
MIT License
85 stars 16 forks source link

Towards 1.0 release #17

Open ronen opened 9 years ago

ronen commented 9 years ago

Here are the outstanding issues & PRs, all of which are reasonable candidates to include in 1.0

But no need to include everything initially if that'd be too much to bite off in one chunk. Most important would be the non-backwards-compatible changes: #9, #10, #16. The others could be layered in later.

jekuno commented 9 years ago

I created a v1.0 branch - let's integrate all pull requests for 1.0 there.

ronen commented 9 years ago

I just added #25 - Viewport selection management.

I didn't mark it as a 1.0 milestone, but I do think it would be nice to include in 1.0 -- it seems like the kind of thing that somebody would quickly ask for. What do you think?

ronen commented 8 years ago

@jekuno, @paresharma where are we on this...? We had a flurry of activity a few months ago but have fallen quiet. Are we ready to release 1.0? What else should be done?

paresharma commented 8 years ago

@ronen I have been meaning to update my status on this for some time now. I have been a bit busy moving base from Oslo back to India. I am settled down now, so hope to post some update real soon.

ronen commented 8 years ago

@paresharma no prob. I've likewise been relocating and otherwise distracted lately. What I really meant to ask was: Are you held up waiting for me to do something? :)

jekuno commented 8 years ago

@paresharma Enjoy your time back in India! :) @ronen I am quite busy an the moment. I did some work in my fork but still did not find the time to cleanup code and write tests.... I hope it gets better the next weeks.

ronen commented 8 years ago

@jekuno also no prob. No specific rush on this, just trying to clean out my open source/github backlog. And of course it'd be a shame if this effort ran dry considering all the work that's gone in to this. But as I said, mostly wanted to make sure that you guys weren't quietly waiting for me to be doing something!

paresharma commented 8 years ago

Thanks @jekuno :)

@ronen I am sure it's just the busy schedule that's holding this project. I am looking forward to putting in some more hours soon. I am with you on this and do not want all the time and efforts to go waste.

And hey happy new year (we're still in the 1st month :smile: ) :tada:

palkan commented 8 years ago

Hi, everyone!

I'd like to help you with the release, 'cause I very interested in such features as expectation labels and render options (e.g. selector: ...). But both PRs look stale.

paresharma commented 8 years ago

@palkan We now have branch called v1.0. We have doing all the work for the release here. Feel free to update the PR or create a new one to this branch.

palkan commented 8 years ago

Hi, guys!

We've extracted image comparison into separate gem - https://github.com/teachbase/imatcher. It supports several comparison algorithms. How about to use it in rspec-page-regression?

ronen commented 8 years ago

Good idea! I'm always in favor of SoC.

BTW a feature to consider for imatcher would be to add region(s) to exclude (as per #7) and/or region(s) to include.

A question this raises though is how to handle the options for imatcher. I can see three choices:

  1. Recognize specific options to match_reference_screenshot to be passed along to imatcher.
    • Advantages: Convenient to the user. Allows match_reference_screenshot options to have different names from imatcher in case that makes most sense.
    • Disadvantages: Updates to imatcher mean that rspec-page-regression needs to be updated
  2. Assume any unrecognized arguments to match_reference_screenshot should be passed to imatcher.
    • Advantages: Convenient to the user. Don't need to update rspec-page-regression if imatcher changes
    • Disadvantages: Could lead to surprising errors. Could lead to conflicts if in the future imatcher gets an option that's the same name as a match_reference_screenshot option
  3. Have a single hash-valued argument match_reference_screenshot matcher: { ... } to pass to imatcher.
    • Advantages: Robust
    • Disadvantages: Not as convenient to the user.

I guess I'd go with both (1) and (3). That is, allow the user to specify the imatcher options wholesale, but also provide specific convenient options that get merged into the hash of (3) before passing to imatcher.

Thoughts?

palkan commented 8 years ago

I prefer (1), the same way we do with render-specific options. And we also should add default imatcher settings to configuration.

P.S. We've started to work on exclude feature)

ronen commented 8 years ago

@palkan yeah i agree (1) is best. and exclude feature will be nice.

chenqingspring commented 7 years ago

Thanks for this great gem! I was a PhantomCss user, working on a RoR project, your gem make me gain a lot of benefits, like:

  1. I can use FactoryGirls to build data within each rack test, rather than seed entire test data.
  2. I can integrate ui test with function test in a same feature, rather than separate them.
  3. I don't need to write deep callbacks with casperjs if my test has multiple steps within single feature.

But, can you please let me know why you guys stop release v1.0? and any thing blocked development?

palkan commented 7 years ago

Hi, @chenqingspring!

You can try our fork (https://github.com/teachbase/rspec-page-regression/tree/use-imatcher) which is based on v1.0. We proposed the PR about a year ago, but...

chenqingspring commented 7 years ago

Hi, @palkan

Thanks for your reply, I'll try your fork soon!

I think this gem did the right thing, but the name rspec-page-regression doesn't describe the core value very well. I use image comparison only for testing css logic, and capybara itself handle the functional testing work. so I think let's why it has less stars than it should..

But, anyway, I'll will continue to use it, and to see if I can do some maintain work. Thanks again!

jekuno commented 7 years ago

@palkan Thanks for your work. I haven't been involved into https://github.com/rprt/rspec-page-regression/pull/31 but I suppose the PR couldn't be merged until now as the build for this PR fails (see the discussion).

@chenqingspring Do you have any suggestion for a better name?

palkan commented 7 years ago

@jekuno I've fixed #31.

chenqingspring commented 7 years ago

Hi, @jekuno I prefer rspec-css-regression or in short rspec-css or rspec-ui-testing, which sells the layout verification in rspec. what's your opinion?

palkan commented 7 years ago

I don't think rspec-css is a good name, 'cause UI is not only CSS.

rspec-ui would be great, but this name is occupied. What about ui-rspec?

ronen commented 7 years ago

Hi all, glad to see this project waking back up!

I prefer rspec-css-regression or in short rspec-css or rspec-ui-testing,

rspec-ui would be great, but this name is occupied. What about ui-rspec?

I don't think "UI" is great because a UI includes input, which this gem doesn't address. This is just about what it looks like.

I think rspec-css* are pretty good, but there could be images involved too.

I think that although rspec-page-regression is boring, it's pretty accurate. Is there something else that's less boring that would also be clear?

rspec-looks ? rspec-looks-regression?

Or forget about being clear and make something snazzy rspec-pagetastic or fun love-your-css? :)

ronen commented 7 years ago

But, can you please let me know why you guys stop release v1.0? and any thing blocked development?

@jekuno what do you think, can we get release v1.0 back on track?

palkan commented 7 years ago

Maybe, rspec-shot or rspec-page-shot to indicate that we're using screenshots.

jekuno commented 7 years ago

@ronen Yes, I think we can get v1.0 back on track. Most work is already done. Checking the list at https://github.com/rprt/rspec-page-regression/milestone/1 it's mainly #22 pending (plus some minor updates for #21). Within the next two weeks I might find some time for #22.

Regarding the name of the gem: IMHO most rspec tests are regression tests as they define how a part of the application should behave and turn from green to red as soon as there is a regression. As regression also is a little bit bulky it might make sense to remove this part from the name of the gem. I like the idea of referring to screenshots in the name of the gem as this is what the gem basically does: Taking screenshots and comparing them with expectations.

@ronen Regarding the term ui: I think this term isn't too bad because we indeed check the complete user interface by taking screenshots (I for example take screenshots of forms and even the appearance of form validation errors as well). If we would agree that rspec-ui is a nice name we might even be able to convince chris-teague to hand over the name to our gem.

Other ideas (quick creative brainstorm): rspec-screenshooter, rspec-screenshots, screenshot-checker, check-the-shot, spot-by-shot, rspec-screencheck, ui-shooter, rspec-eye, eye-of-rspec, rspec-shooting-star, browser-shots, browser-eye (or any other nice combinations of those terms)

ronen commented 7 years ago

@jekuno yes good point re regression. Though I still think ui would imply testing the behavior in response to input events, not just the screenshot.

I like the names that have shot or screenshot in them. Those also line up with the usage expect(page).to match_reference_screenshot.

Adding another some more terms to the brainstorm: rspec-mugshot, hit-me-with-your-best-shot, straight-shooter

jekuno commented 7 years ago

shot is short but also ambiguous whereas screenshot is longer but clearer.

Another made-up word: rspec-greenshot (green tests + screenshot)

ronen commented 7 years ago

rspec-greenshot is pretty cute

chenqingspring commented 7 years ago

Good to know this project is moving forward!!

I created a pull request #39 only for updating README, but build failed...

palkan commented 7 years ago

I created a pull request #39 only for updating README, but build failed...

There is a problem with activesupport (breaking changes in 5.x).

@ronen @jekuno I've fixed the problem in #31 (by getting rid of AS completely https://github.com/rprt/rspec-page-regression/pull/31/commits/0887f1cf69e8c3b3f58d333b499252cf245fd04f), would you like me to extract it into a separate PR? Or, maybe, we're ready to merge #31?

jekuno commented 7 years ago

I'm testing #31 right now

chenqingspring commented 7 years ago

Hi guys, any progress?