refactor: remove more test order dependence #4273
Conversation
elasticspoon
force-pushed
the
fix-more-order-dependent-tests
branch
from
662fe00
to
f8b008f
Compare
As discussed here rubyforgood#4264, some tests modify global variables and expect them to be reset before each test. This makes it more difficult to move forward with removal of pre-seeding. This commit refactors certain tests to be less brittle to changes in global state and to rely less on assumptions on global state. Ex: not assuming that there are 0 records of a certain type when a test starts, etc.
elasticspoon
force-pushed
the
fix-more-order-dependent-tests
branch
from
f8b008f
to
5e64fbb
Compare
elasticspoon
added
the
BLOCKER
| let!(:today) { create(described_class.to_s.underscore.to_sym, date_field.to_sym => Time.zone.local(2019, 7, 31)) } | ||
| let!(:very_old) { create(described_class.to_s.underscore.to_sym, date_field.to_sym => Time.zone.local(2000, 7, 31), :organization => @organization) } | ||
| let!(:recent) { create(described_class.to_s.underscore.to_sym, date_field.to_sym => Time.zone.local(2019, 7, 24), :organization => @organization) } | ||
| let!(:today) { create(described_class.to_s.underscore.to_sym, date_field.to_sym => Time.zone.local(2019, 7, 31), :organization => @organization) } |
There was a problem hiding this comment.
Why are we adding instance variables here when we're removing them everywhere else?
| let(:item2) { create(:item, name: "Crap item") } | ||
| let(:partner1) { create(:partner, name: "This Guy", email: "thisguy@example.com") } | ||
| let(:partner2) { create(:partner, name: "Not This Guy", email: "ntg@example.com") } | ||
| let(:item1) { create(:item, name: "Good item", item_category: item_category, organization: @organization) } |
| item = Item.alphabetized.first | ||
| select @storage_location.name, from: "purchase_storage_location_id" | ||
| expect(page).to have_content(@item.name) | ||
| select @item.name, from: "purchase_line_items_attributes_0_item_id" |
There was a problem hiding this comment.
Same. In general I'm confused here why some tests are removing instance variables and others are adding them.
There was a problem hiding this comment.
Basically I wanted to make minimal changes. A lot of the order dependence issues stemmed from needing to set organization: on things. Thus, where possible I set it with the instance variable organization but since the db is not getting reset within a spec file some of the specs modify @organization and cause other ones to fail.
In general I could remove all the changes to @organization with a created organization but I think that will expand the PR a decent amount. They are also really to find and replace later.
EX: with some of the shared examples, I have to change all the parts that use it if I move off instance variables.
There was a problem hiding this comment.
I basically went:
- does this work? If yes leave
- does this work if I add instance variables to it? If yes add them
- otherwise replace instance variables with created ones.
elasticspoon
force-pushed
the
fix-more-order-dependent-tests
branch
from
bd791be
to
6dea724
Compare
|
...dangit - @elasticspoon there's a conflict now 😁 |
|
@dorner fixed. I'll rebase the other PR and mark it as ready for review whenever this is merged. |
|
Great, thanks! |
|
@elasticspoon: Your PR |
As discussed here #4264, some tests modify global variables and expect them to be reset before each test. This makes it more difficult to move forward with removal of pre-seeding.
This PR refactors certain tests to be less brittle to changes in global state and to rely less on assumptions on global state. Ex: not assuming that there are 0 records of a certain type when a test starts, etc.
Also spec files starting with
describehave been changed to start withRspec.describe. This makes future greping much simpler.