-
-
Notifications
You must be signed in to change notification settings - Fork 572
bug-5152 Adds test coverage to transfer export, which already include… #5202
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
bug-5152 Adds test coverage to transfer export, which already include… #5202
Conversation
dorner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion.
| let(:unused_item) { create(:item, name: "Unused Item", organization: organization) } | ||
| let(:generated_csv_data) do | ||
| # Force unused_item to be created first | ||
| unused_item |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be simpler to replace let with let! on line 105, and then you don't have to do this? Same below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point and yes, that works -- definitely still re-learning Rspec conventions.
|
Thanks! |
|
@bsbonus: Your PR |
rubyforgood#5202) * bug-5152 Adds test coverage to transfer export, which already includes inactive items * bug-5152 cleans up rspec file to use let
…s inactive items
Checklist:
X I have performed a self-review of my own code,
X I have commented my code, particularly in hard-to-understand areas,
X I have made corresponding changes to the documentation,
X I have added tests that prove my fix is effective or that my feature works,
X New and existing unit tests pass locally with my changes ("bundle exec rake"),
X Title include "WIP" if work is in progress.
X I acknowledge that I will not force push my branch once reviews have started.
Resolves #5152 - partially, see main thread
Description
Adds missing test coverage to existing export
Type of change
How Has This Been Tested?
Just running rspec.