Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Establish reliable CI pipeline #287

Open
3 tasks
kettanaito opened this issue Sep 19, 2018 · 5 comments
Open
3 tasks

Establish reliable CI pipeline #287

kettanaito opened this issue Sep 19, 2018 · 5 comments
Assignees
Labels
CRITICAL help wanted Extra attention is needed tests
Milestone

Comments

@kettanaito
Copy link
Owner

kettanaito commented Sep 19, 2018

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 kettanaito added this to the 1.x milestone Sep 19, 2018
@kettanaito kettanaito changed the title Handle explicit timeouts in integration tests Establish reliable CI pipeline Oct 7, 2018
@kettanaito
Copy link
Owner Author

#268 makes CI pipeline incredibly unreliable.

@kettanaito
Copy link
Owner Author

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

@kettanaito kettanaito mentioned this issue Oct 18, 2018
13 tasks
@kettanaito
Copy link
Owner Author

kettanaito commented Oct 22, 2018

  • 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.
  • Try custom assertions like .should($el => ...), which may grant more control over re-trying a certain assertion

@kettanaito kettanaito self-assigned this Oct 22, 2018
@kettanaito
Copy link
Owner Author

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 kettanaito modified the milestones: 1.x, Optimization Nov 2, 2018
@kettanaito
Copy link
Owner Author

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

@kettanaito kettanaito added the help wanted Extra attention is needed label Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CRITICAL help wanted Extra attention is needed tests
Projects
None yet
Development

No branches or pull requests

1 participant