percy / percy-ember

Ember addon for visual regression testing with Percy
https://docs.percy.io/docs/ember
MIT License
73 stars 44 forks source link

Percy not working for <input> #6

Closed cbroeren closed 7 years ago

cbroeren commented 8 years ago

We've implemented percy for many editors in our application. As it says, it's an editor, so it contains many input fields. For that, we use the html tag with our components:

 export default Component.create({
  tagName:    'input',
  attributeBindings: ['type', '_value:value'],
  type: 'text',
  ...
});

Unfortunately, the inputs are shown on the snapshots, but without the value. With the same construction, text-areas aren't shown with the value either. The text-area just contains '<!---->'.

Isn't it possible to snapshot inputs?

(Also, because of those empty inputs, I implemented percy on our own addon we use for the inputs. There the percy build wasn't finalized. It creates a build, sends the first 4 snapshots and then... nothing. Even if I run it locally. When I run it locally and just run 1 test, it will be finalized. Also with the tests that was not in the first 4 snapshots. Any ideas on this?)

fotinakis commented 8 years ago

This should just work, it is definitely possible to snapshot inputs. A few questions to help me track this down: What browser are you using in CI (PhantomJs? Chrome?)? Are these acceptance tests or component tests?

For the second issue, I assume it is because you are using ember-try in that addon environment and so multiple builds are created. We need to improve our ember-try/addon support. You can try to do the same thing we did here and disable percy for all but one build: https://github.com/percy/ember-percy/blob/master/.travis.yml#L14

cbroeren commented 8 years ago

Hi Mike,

Firstly thanks for your answer. To begin at the end, I am using ember-try, but I already set it practically like your example. I set the PERCY_ENABLE=0 on Travis by default, and added PERCY_ENABLE=1 to just one build. I even tested it by adding the PERCY_ENABLE=0 to every other build directly in the travis.yml.

For the real issue, we're testing on Chrome. When I implemented it into the addon, I've used integration tests. In our own application, we've implemented percy into our acceptance tests. So basically we've done both.

https://percy.io/Fabriquartz/ember-railio-form-components https://github.com/Fabriquartz/ember-railio-form-components/pull/31 is the repository of the addon I'm talking about. The application itself is a private one.

fotinakis commented 8 years ago

Hey @cbroeren — we updated things last week to fix a bunch of bugs with setting commit statuses, can you try building these again?

As for the input issue, I haven't had time to look into it very deeply, but I suspect it relates to if the input value is set on the element but is not reflected in the DOM (if the value is not present in the DOM, Percy cannot capture it).

fotinakis commented 8 years ago

I believe I've figured out the input problem — in general, DOM elements have properties and attributes. They are not the same—typing into an input or textarea sets a property on the element, not the attributes. Currently ember-percy's DOM snapshots only serializes the DOM with its attributes, so we lose the value of inputs and textboxes (and probably other form elements like checkboxes).

The fix for this is to write some jQuery in our DOM snapshot logic that enumerates inputs on the page and stuff their current property value into their attribute value.

bennettmt commented 7 years ago

Hello Mike,

I am also having issues with inputs not reflecting their value in snapshots. Does the open PR still require work in order to be merged?

Thank you

fotinakis commented 7 years ago

Hey @bennettmt, sorry, the PR still needs work to be merged. We could reboot it and limit the scope to just fix input boxes (instead of including things like radio buttons and others). That would probably be a good v1 fix for it.

fotinakis commented 7 years ago

Fixed by #29!

cbroeren commented 7 years ago

Many many thanks to @bennettmt for fixing this!