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

Upgrade to Rails 5 #435

Merged
merged 25 commits into from Jan 12, 2016

Conversation

Projects
None yet
2 participants
@schneems
Member

schneems commented Jan 12, 2016

I picked off where #427 left off. There's a few more gems that needed to be updated and some minor code changes.

I'm still getting some random failures that I think are omniauth related. Still investigating.

@schneems schneems deployed to codetriage-staging-pr-435 Jan 12, 2016 Active

@schneems schneems force-pushed the schneems/rails5 branch from a5f4830 to 0667916 Jan 12, 2016

@schneems schneems had a problem deploying to codetriage-staging-pr-435 Jan 12, 2016 Failure

@schneems schneems deployed to codetriage-staging-pr-435 Jan 12, 2016 Active

@schneems

This comment has been minimized.

Member

schneems commented Jan 12, 2016

I'm getting this error locally too

ActionView::Template::Error: nil is not a valid asset source
    app/views/application/_nav.html.slim:9:in `_app_views_application__nav_html_slim__1550834241246068923_79168440'
    app/views/layouts/application.html.slim:7:in `_app_views_layouts_application_html_slim__587618798759587358_37113340'
    test/test_helper.rb:86:in `login_via_github'
    test/integration/maintaining_repo_subscriptions_test.rb:7:in `triage_the_sandbox'
    test/integration/maintaining_repo_subscriptions_test.rb:35:in `block in <class:MaintainingRepoSubscriptionsTest>'

This only happens when we're using the login_via_github test helper which uses the omniauth stubs. Sometimes avatar_url is nil, sometimes it isn't. I added logging and sometimes current_user is nil and sometimes it isn't. Really not sure where this is coming from.

@prathamesh-sonpatki

This comment has been minimized.

Member

prathamesh-sonpatki commented Jan 12, 2016

I am getting a different failure on this branch -

F

Failure:
ReposControllerTest#test_trying_to_create_repo_without_logged_in_will_redirect_to_login_page:
Expected response to be a redirect to <http://test.host/users/auth/github?origin=%2Frepos%3Frepo%255Bname%255D%3Dcodetriage%26repo%255Buser_name%255D%3Dcodetriage> but was a redirect to <http://test.host/users/auth/github?origin=%2Frepos>.
Expected "http://test.host/users/auth/github?origin=%2Frepos%3Frepo%255Bname%255D%3Dcodetriage%26repo%255Buser_name%255D%3Dcodetriage" to be === "http://test.host/users/auth/github?origin=%2Frepos".

Investigating more.

@schneems schneems deployed to codetriage-staging-pr-435 Jan 12, 2016 Active

Fix test
Not sure if this is a change in the test runner or in Rails. Here's what's happening. We are storing `request.fullpath` and redirecting to it later, that is what this test is testing. Previously the `@fullPath` would also include query params, now it only shows the path:

```
@fullPath="/repos"
```

The other elements are sent in the request but they're in either 

```
action_dispatch.request.parameters"=>{"repo"=>{"name"=>"codetriage", "user_name"=>"codetriage"}, "controller"=>"repos", "action"=>"create"}
```

or 


```
"rack.request.form_hash"=>{"repo"=>{"name"=>"codetriage", "user_name"=>"codetriage"}}
```

or

```
"rack.request.form_vars"=>"repo%5Bname%5D=codetriage&repo%5Buser_name%5D=codetriage"
```

This fixes the test and does ensure that we're redirecting correctly, but it does mean that any redirects will not have data from a form post. I think this makes sense since you can't redirect to a POST, only a GET.

cc/ @prathamesh-sonpatki

@schneems schneems deployed to codetriage-staging-pr-435 Jan 12, 2016 Active

@schneems

This comment has been minimized.

Member

schneems commented Jan 12, 2016

Branch is green, I commented out teaspoon tests. We should revisit our javascript testing story. I have to run to a meeting. I'm fine to deploy and work on bugs when I get back.

@prathamesh-sonpatki

This comment has been minimized.

Member

prathamesh-sonpatki commented on test/test_helper.rb in 37dae4c Jan 12, 2016

👍, In Rails 4 the avatar_url was nil, but asset_url did not raise error for nil source in Rails 4.

It was added in rails/rails#20669

@prathamesh-sonpatki

This comment has been minimized.

Member

prathamesh-sonpatki commented on a4a118f Jan 12, 2016

👍 This might be from Rack but not sure.

schneems added a commit that referenced this pull request Jan 12, 2016

@schneems schneems merged commit 7a8fe29 into master Jan 12, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@schneems

This comment has been minimized.

Member

schneems commented Jan 12, 2016

Rails 5 beta is live on production

@prathamesh-sonpatki prathamesh-sonpatki deleted the schneems/rails5 branch Jan 23, 2016

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