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

Mutlitenancy in test #1105

Merged
merged 6 commits into from May 12, 2017
Merged

Mutlitenancy in test #1105

merged 6 commits into from May 12, 2017

Conversation

atz
Copy link
Contributor

@atz atz commented May 11, 2017

See #989 where this was all debated over being merged into a branch that was thrown away. But we actually want this.

mjgiarlo
mjgiarlo previously approved these changes May 11, 2017
@atz
Copy link
Contributor Author

atz commented May 12, 2017

Our new strictness makes fake tenant accounts inadequate. Trying to fix.

atz added 5 commits May 11, 2017 19:12
This will break, but we should push through to make this the default.
These would only affect Travis.
**:singletenant**

This is effectively the reverse of `:multitenant`.  It is the cheapest
way to fix existing tests that would fail under multitenant default.

Singletenant tests

- Feature test default singletenant
- riiif specs are singletenant
- appearances_controller_spec.rb
- groups_controller_spec.rb

That's how all these were written.

**faketenant**

Specifically tests that don't care about single or multi-tenancy, they
just want there to seem like there is a consistent real tenant.

Give all controller tests faketenant unlesss otherwise specified
Because the tenant is going to be required for basic routing.
@atz
Copy link
Contributor Author

atz commented May 12, 2017

All fixed... except for one failure in "push".

@mjgiarlo
Copy link
Member

mjgiarlo commented May 12, 2017

I restarted the "push" build. In the meantime, here was the one failure:

  1) SolrEndpoint#connection returns the initialized connection (without an adapter option)
     Failure/Error: af_defaults = ActiveFedora::SolrService.instance.conn.options
     
       RSolr received :connect with unexpected arguments
         expected: ({"read_timeout"=>120, "open_timeout"=>120, "url"=>"http://example.com/solr/"})
              got: ({:read_timeout=>120, :open_timeout=>120, :url=>"http://127.0.0.1:8985/solr/hydra-test"})
       Diff:
       @@ -1,2 +1,4 @@
       -[{"read_timeout"=>120, "open_timeout"=>120, "url"=>"http://example.com/solr/"}]
       +[{:read_timeout=>120,
       +  :open_timeout=>120,
       +  :url=>"http://127.0.0.1:8985/solr/hydra-test"}]
       
        Please stub a default value first if message might be received with other args as well. 
     # ./app/models/solr_endpoint.rb:14:in `connection_options'
     # ./app/models/solr_endpoint.rb:8:in `connection'
     # ./spec/models/solr_endpoint_spec.rb:12:in `block (3 levels) in <top (required)>'
     # ./spec/models/solr_endpoint_spec.rb:24:in `block (3 levels) in <top (required)>'

@atz @jcoyne @cbeer I have seen this one before, but it it usually resolved by restarting the build. That said, do y'all think this is something we should be worrying about? I ask because we have Solr connection issues in demo-land -- maybe they're not connected, though.

@atz
Copy link
Contributor Author

atz commented May 12, 2017

Why can't I get this locally though?

@mjgiarlo mjgiarlo merged commit d2e9018 into master May 12, 2017
@mjgiarlo mjgiarlo deleted the mutlitenancy_in_test branch May 12, 2017 16:38
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.

None yet

2 participants