Skip to content
This repository has been archived by the owner on Jul 24, 2020. It is now read-only.

[416] Add more integration tests #1219

Merged
merged 1 commit into from
Apr 24, 2015
Merged

[416] Add more integration tests #1219

merged 1 commit into from
Apr 24, 2015

Conversation

orenyk
Copy link
Contributor

@orenyk orenyk commented Apr 3, 2015

Resolves #416

@orenyk
Copy link
Contributor Author

orenyk commented Apr 3, 2015

This has been failing some specs, again, so I'm not sure if this is actually ready. I'll double check after the weekend.

@orenyk
Copy link
Contributor Author

orenyk commented Apr 7, 2015

Note that the integration tests written for #987 / #1206 are clunky since that branch doesn't include the feature helpers in this one. Once #1206 is merged in to master, this should be rebased onto master and spec/features/eq_model_spec.rb should be edited appropriately.

@orenyk
Copy link
Contributor Author

orenyk commented Apr 8, 2015

Hopefully resolved the failures by rearranging how we ran our app_setup step for feature tests, Travis will let us know 😄.

EDIT Woot green build!

@@ -87,17 +81,14 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

  • this is just a marker for myself *

@orenyk orenyk force-pushed the 416_more_integration_tests branch from 0b0babd to 3c34bdd Compare April 16, 2015 23:58
@orenyk
Copy link
Contributor Author

orenyk commented Apr 17, 2015

Ok, I updated the specs from #987 so they should be much cleaner now, hopefully this will still be green.

expect(query.count).to eq(1)
expect(query.first.start_date).to eq(Time.zone.today)
expect(query.first.due_date).to eq(bad_due_date)
expect(query.first.approval_status).to eq('auto')
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will change when #462 is merged in

expect(query.count).to eq(1)
expect(query.first.start_date).to eq(Time.zone.today)
expect(query.first.due_date).to eq(due_date)
expect(query.first.approval_status).to eq('auto')
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will change when #462 is merged in

@orenyk orenyk force-pushed the 416_more_integration_tests branch 3 times, most recently from c17d32a to f6d6a83 Compare April 20, 2015 04:49
@orenyk
Copy link
Contributor Author

orenyk commented Apr 20, 2015

Ok, this is ready for a re-review; we should also decide if we're going to rework the for_eq_model scope before we merge this in.

@orenyk orenyk force-pushed the 416_more_integration_tests branch from f6d6a83 to e161e6d Compare April 21, 2015 04:29
@orenyk
Copy link
Contributor Author

orenyk commented Apr 21, 2015

I updated the for_eq_model scope not to call finalized, then updated most of the scope calls to explicitly call finalized as well. There were a few places where it was already calling status-based scopes (either active or overdue) so I didn't need to specify finalized. All specs passed locally and on Travis, please re-review!

@esoterik
Copy link
Collaborator

need to check how we're doing reservation renewal math for showing the renew button

@esoterik
Copy link
Collaborator

there should be tests for renewal eligibility

Resolves #416
- add tests for Reservation actions (creation, equipment handling, renewal)
- add tests for Rails Admin routes
- resolve issues with renewals and add specs (see #1218)
- add numerous helpers for integration tests
- misc cleanup of other tests
- resolve deprecation warning reintroduced by #1081
- clean up tests from #987
- refactored the for_eq_model Reservation scope not to call .finalized
@orenyk orenyk force-pushed the 416_more_integration_tests branch from e161e6d to 4a06ca1 Compare April 24, 2015 00:42
@orenyk
Copy link
Contributor Author

orenyk commented Apr 24, 2015

Ok, updated!

@esoterik
Copy link
Collaborator

Everything seems to work as expected!

@orenyk
Copy link
Contributor Author

orenyk commented Apr 24, 2015

Great, merging!

orenyk added a commit that referenced this pull request Apr 24, 2015
@orenyk orenyk merged commit c6635c5 into master Apr 24, 2015
@orenyk orenyk deleted the 416_more_integration_tests branch October 27, 2015 19:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration Tests
3 participants