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

increase test coverage #966

Merged
merged 1 commit into from
Nov 27, 2022
Merged

Conversation

yksflip
Copy link
Member

@yksflip yksflip commented Oct 31, 2022

this pr adds more integration and model tests to increase the test coverage.

Copy link
Member

@paroga paroga left a comment

Choose a reason for hiding this comment

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

many thanks for the additional test. i think there are only a few stylistic issues. i know that we don't have a exact style guide and the current style is no consistent, but it would be great to use the ruby style guide where possible or stay at least consistent with the style of a file

spec/factories/delivery.rb Outdated Show resolved Hide resolved
spec/factories/delivery.rb Outdated Show resolved Hide resolved
spec/integration/supplier_spec.rb Outdated Show resolved Hide resolved
visit suppliers_path
click_link I18n.t('ui.edit')
fill_in I18n.t('activerecord.attributes.supplier.name'), with: new_name
click_button 'Update Supplier'
Copy link
Member

Choose a reason for hiding this comment

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

I18n?

Copy link
Member Author

Choose a reason for hiding this comment

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

coud not find the right i18n key ... find('input[type="submit"]').click should be okay too

spec/integration/supplier_spec.rb Outdated Show resolved Hide resolved
spec/models/supplier_spec.rb Outdated Show resolved Hide resolved
spec/models/supplier_spec.rb Outdated Show resolved Hide resolved
spec/models/group_order_article_spec.rb Outdated Show resolved Hide resolved
spec/integration/home_spec.rb Outdated Show resolved Hide resolved
spec/factories/supplier.rb Outdated Show resolved Hide resolved
@yksflip
Copy link
Member Author

yksflip commented Oct 31, 2022

thanks for your comments!
I'm a bit sorry, I totally forgot to run rubocop before pushing ...

@yksflip yksflip force-pushed the 8_increase_test_coverage branch 2 times, most recently from 893021c to ad03caf Compare October 31, 2022 21:42
@yksflip
Copy link
Member Author

yksflip commented Oct 31, 2022

mh so rubocop want's

spec/integration/home_controller_spec.rb:3:1: C: [Correctable] RSpec/Capybara/FeatureMethods: Use describe instead of feature.
feature 'my profile page' do

but if i do so the login function can´t be find anymore ... any ideas?

spec/models/article_spec.rb Outdated Show resolved Hide resolved
spec/integration/home_controller_spec.rb Outdated Show resolved Hide resolved
spec/integration/supplier_spec.rb Outdated Show resolved Hide resolved
@paroga
Copy link
Member

paroga commented Nov 1, 2022

but if i do so the login function can´t be find anymore ... any ideas?

add it to the .rubocop_todo.yml, like we did for the other tests

could you please make your commit message a bit more clean and add what test (or at least in which area) are added in the commit. also please be consistent with the Co-authored-by. upper case C seams to be the common way to write it

@yksflip yksflip force-pushed the 8_increase_test_coverage branch from ad03caf to 44c7f33 Compare November 2, 2022 09:02
@yksflip
Copy link
Member Author

yksflip commented Nov 2, 2022

thanks for your hints! I hope i covered them all now :)

Copy link
Member

@paroga paroga left a comment

Choose a reason for hiding this comment

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

super thanks for all the changes, could you make one small additional change for rubocop?

and sorry for not being more clear in the first step about the commit message: it would be great if the first line of the commit message would contain more information. I expect additional PRs about the test coverage in the near future and it would be great if they are at least a little bit distinguishable there 😉

.rubocop_todo.yml Outdated Show resolved Hide resolved
This commit adds new tests for a better coverage.

integration/
  * home
  * supplier

models/
  * article
  * delivery
  * group_order_article
  * supplier

also adds a new factory for delivery

Co-authored-by: viehlieb <pf@pragma-shift.net>
Co-authored-by: Tobias Kneuker <tk@pragma-shift.net>
@yksflip yksflip force-pushed the 8_increase_test_coverage branch from 44c7f33 to 9fa58ff Compare November 3, 2022 19:18
@yksflip
Copy link
Member Author

yksflip commented Nov 21, 2022

ping :)

@paroga paroga merged commit c6560e0 into foodcoops:master Nov 27, 2022
@yksflip yksflip added this to the 4.8 milestone Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants