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

Some hygeine so Travis tests can pass #113

Conversation

bibliotechy
Copy link
Contributor

Currently, running the tests on master on Travis for this gem fail with two distinct errors:

  1. ActionView::Template::Error: Autoprefixer doesn’t support Node v8.12.0. Update it. - 9 tests fail with this issue.
  2. NoMethodError: undefined method 'search_state_class' for #<ActionView::TestCase::TestController:0x00000000094b15a0> - 2 tests fail with this issue.

Additionally, there are some warnings about gems being found in multiple sources:

Warning: the gem 'bindex' was found in multiple sources.
Installed from: https://rubygems.org/
Also found in:
  * http://rubygems.org/
You should add a source requirement to restrict this gem to your preferred source.

This PR fixes those errors by:

  1. Updating the TRavis config to use Node 12
  2. Updating the render_constraints_override spec to work with with changes introuced in Blacklight 7.8.0, by commmit projectblacklight/blacklight@c11db26

It eliminates the multiple source gem warnings by updating updating the rubygems url to https.

Currently, running the tests on master on Travis for this gem fail with two distinct errors:
1. `ActionView::Template::Error: Autoprefixer doesn’t support Node v8.12.0. Update it.`  - 9 tests fail with this issue.
2. `NoMethodError: undefined method `search_state_class' for #<ActionView::TestCase::TestController:0x00000000094b15a0>` - 2 tests fail with this issue.

Additionally, there are some warnings about gems being found in multiple sources:
```
Warning: the gem 'bindex' was found in multiple sources.
Installed from: https://rubygems.org/
Also found in:
  * http://rubygems.org/
You should add a source requirement to restrict this gem to your preferred source.
```

This PR fixes those errors by:

1) Updating the TRavis config to use Node 12
2) Updating the render_constraints_override spec to work with  with changes introuced in Blacklight 7.8.0, by commmit projectblacklight/blacklight@c11db26

It eliminates the multiple source gem warnings by updating updating the rubygems url to https.
@bibliotechy
Copy link
Contributor Author

One of the three builds failed because SolrWrapper timed out on download. Could someone with travis permissions try re-running this build? https://travis-ci.org/github/projectblacklight/blacklight_advanced_search/jobs/729714069

.travis.yml Outdated
@@ -1,6 +1,9 @@
notifications:
email: false

before_install:
nvm install 12
Copy link
Contributor

Choose a reason for hiding this comment

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

In other projects we've used the xenial distribution on travis, which has Node 12.18.2 already.

Have you tried that or would you consider using that as opposed to updating node as part of the build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkeck Happy to try a newer distro. I do note that core Blacklight uses bionic.

https://github.com/projectblacklight/blacklight/blob/master/.travis.yml#L1

Is there a specific preference for xenial for this, or should I match core BL?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say +1 to bionic which should give you the updated node automagically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I hadn't noticed that we weren't even defining a dist. If Bionic gives us the right node, then 👍.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkeck @mejackreed Bionic work great. Thanks!

Copy link
Contributor

@jkeck jkeck left a comment

Choose a reason for hiding this comment

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

💨

@jkeck jkeck merged commit 4173fef into projectblacklight:master Sep 24, 2020
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.

3 participants