Skip to content

Conversation

@sauloperez
Copy link
Collaborator

@sauloperez sauloperez commented Aug 1, 2017

El viernes estuve trabajando en un pequeño refactor de transferencias de tiempo entre miembros y organizaciones y me vi constantemente probando las transferencias manualmente. Aunque tenemos alguno tests justamente tuve que añadir tests de controller para los actions que estoy tocando (pull request en breve).

Sin embargo, para ir más ágil pensé en automitzar el test manual de transferir y así avanzar más rápido en el refactor sin romper nada. Esto al darnos más seguridad de que las cosas no se rompen facilitará las contribuciones esporádicas y quitará algo de trabajo a @sseerrggii .

Esta PR añade las gemas necesarias para empezar a tener unos spec/features/ con los que podamos garantizar que las features esencial y críticas de la app funcionan.

De momento solo añado un test para login y otro para logout. Aún tengo que averigar como hacer tests ejecutando JS.

@sauloperez sauloperez force-pushed the feature-specs branch 4 times, most recently from a712df2 to 07b4e61 Compare August 7, 2017 13:00
@sauloperez sauloperez added the wip label Aug 7, 2017
This sets up capybara with the new headless chrome driver and adds the
necessary requires in order to run single spec file. So far, there was
no other way than doing `bundle exec rake` due to missing requires.
The headless chromedriver requires Chrome to be installed. See
https://robots.thoughtbot.com/headless-feature-specs-with-chrome#on-ci
for details.
We had `use_transactional_fixtures` enabled but no fixtures where
defined in spec/fixtures. This prevented Capybara tests with javascript
to work. This is explained in detail in
https://github.com/DatabaseCleaner/database_cleaner#rspec-with-capybara-example.
context 'with a valid password' do
it 'signs the user in' do
sign_in_with(user.email, 'papapa22')
expect(page).to have_content(I18n.t('application.navbar.sign_out'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.sign_out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it checks that the log out link is present. That means we succesfully logged in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit misleading IMHO, maybe we should specify that in the example description or find another way to be check that the user is logged in


# hack alert! there is no translation for this string. How we build the
# copy then?
click_button 'Crear Transferencia'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which language are we executing feature specs? We should use only one, english IMHO.

Copy link
Collaborator Author

@sauloperez sauloperez Aug 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't touched any language setting so I guess some sort of default. We'll need to check it.

require 'capybara/rails'
require 'database_cleaner'
require 'fabrication'
require 'selenium/webdriver'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this for unit tests too?

Copy link
Collaborator Author

@sauloperez sauloperez Aug 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, and capybara/rails neither but where do you want to require them? remember a bundle exec rake should work as well (it's what CI does).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to create a feature_spec_helper.rb with this kind of stuff, but if I'm OK if we leave it here.

Document.create(label: "t&c") do |doc|
doc.title = "Terms and Conditions"
doc.content = "blah blah blah"
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the T&C in spec_helper are not enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I fixed it with the last push and now I need to remove this.

@sauloperez
Copy link
Collaborator Author

I give up, at least for now. Feature testing with angular it's painful and although some people succeeded it appears not to be a common approach.

The main problem is that due to my lack of knowledge of angular, I can't manage to wait for it to "compile" the views. The DOM elements are still hidden when the test is executed and Capybara doesn't like that. Even forcing it to find hidden elements, I get Selenium::WebDriver::Error::ElementNotVisibleError errors.

Once we remove angular (I think it brings little to no value now) from the app we will be in a much better position.

@sauloperez sauloperez closed this Aug 16, 2017
@enricostano
Copy link
Contributor

😭

@sauloperez sauloperez mentioned this pull request Mar 1, 2018
@Morantron Morantron self-assigned this Mar 14, 2018
@Morantron
Copy link
Collaborator

Fixing provisioning issues here coopdevs/timeoverflow-provisioning#40

And adding chromedriver to devenv here

I'm now investigating DatabaseCleaner strategies. Looks like it's cleaning too compulsively :trollface: By the time the sign in request reaches the controller, the user is gone from DB 💥 Tried removing DatabaseCleaner and the sign in spec works ( assertion needs to be fixed )

See screenshot from capybara here coopdevs/timeoverflow-provisioning#41

wtf

@sauloperez
Copy link
Collaborator Author

🙈 we're pretty late to the capybara party so there must be some documentation to play it well with database cleaner.

@Morantron Morantron mentioned this pull request Mar 14, 2018
@Morantron
Copy link
Collaborator

okiii, let's continue in #333 💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants