kettanaito / react-advanced-form

Functional reactive forms. Multi-layer validation, custom styling, field grouping, reactive props, and much more.

https://redd.gitbook.io/react-advanced-form

MIT License

217 stars 24 forks source link

Establish reliable CI pipeline #287

Open kettanaito opened 6 years ago

kettanaito commented 6 years ago

Environment

react-advanaced-form: 1.4.1

What

I suggest to consider the necessity of explicit cy.wait in the integration tests.

Why

Using timeouts may be suitable in some cases, but I am growing worried that the library responsiveness may be in risk. The reason of using timeouts is (probably) related to the form not being fully bootstrapped before trying to run assertions on it, or its fields.

How

[ ] Investigate why timeout is needed and what is helps to avoid
[ ] Get rid of timeout / (Alternatively) unify and ensafe timeouts
[ ] Add responsiveness test

This is also a task to establish a reliable CI pipeline. I hate to see current setup producing falsy negative results during integration tests, this is unacceptable. Need to figure out why this happens, and how to eradicate it at its core.

kettanaito commented 6 years ago

268 makes CI pipeline incredibly unreliable.

kettanaito commented 6 years ago

Consider rendering scenario anew for each test, instead of relying on interactive methods like this.form.reset. This can eliminate timeout necessity issue.

kettanaito commented 6 years ago

[x] Try to assert against window.form, i.e. accessing internal state and asserting returns of the methods. This won't be integration tests per design, but if this guarantees reliability (as opposed to now), I would go for it. This is also how integration tests were written previously.
[x] Try custom assertions like .should($el => ...), which may grant more control over re-trying a certain assertion

kettanaito commented 6 years ago

One of the reasons the current integration tests are not reliable may be related to the side-effects in Field.componentWillReceiveProps. Having side-effects in that lifecycle method is a no-op, and it can cause unreproducible buggy behavior.

I have removed integration tests from the build pipeline for now. Tests still run on local to ensure nothing is broken. I think Form-Field communication needs to be refined to remove mentioned side-effects, and be more bulletproof. It may be a good opportunity for #232.

kettanaito commented 6 years ago

The reliability of the CI is highly dependent on the #324.