samvera-deprecated / sufia

[DEPRECATED] Sufia: a fully featured, flexible Samvera repository front-end.
http://sufia.io/
Other
111 stars 78 forks source link

WIP get travis green #3183

Closed jrochkind closed 6 years ago

jrochkind commented 6 years ago

With no code changes in this repo, travis on master went from green to red between:

So the change is likely in a new version of some dependency(ies) allowed by spec.

unclear why only tests under rails 5 fail and not under rails 4.2.

Investigating to get green, with as few changes as possible. In some cases it may be acceptable to lock to older dependency instead of allowing new that breaks. In some cases, a minor release of an upstream dependency may be introducing backwards-break change, which should be reported.

jrochkind commented 6 years ago

On travis, it's trying to download solr 6.5.0 -- I'm not really sure why it's doing this, what configuration there says to download that solr instead of the latest. I can't find anything.

On my local computer if I run tests, I get latest solr 7.

But anyway, on travis trying to download solr 6.5.0, it's being rate limited by apache unfortunately. So we've got to wait until tomorrow to see if tests pass. :(

jrochkind commented 6 years ago

So there is the problem everyone in other samvera projects is having with trying to download a solr from apache and getting rate limited. I am getting this to be green by just waiting for a new quota period and manually rebuilding.

It will likely cause problems in future travis builds, but it's really unclear how to fix it. It probably requires changes to the solr_wrapper_test.yml file, whch in this case is generated by an active_fedora generator, that is called by a hydra-head generator, that is (I think? might be more intervening things) called by a curation_concerns generator, that is called by the sufia install generator, that is called by the engine_cart set up test app generator. Phew. Those layers really don't make things easy.

I guess the hacky way to fix it would be to just have the sufia test app generator generate it's own solr_wrapper_test.yml on top of what AF is putting there, with desired parameters to get around the apache quota (either download from princeton mirror, or use travis caching, or both). I honestly can't think of a better way. I think it's out of scope for this particular issue/PR though.

RudyOnRails commented 6 years ago

@jrochkind just curious, could you share the travis line for:

On travis, it's trying to download solr 6.5.0 -- I'm not really sure why it's doing this, what configuration there says to download that solr instead of the latest. I can't find anything.

jrochkind commented 6 years ago

@RudyOnRails Ah, thanks for the sanity check. It looks like that isn't currently the problem.

Not sure what the problem currently is. I'll try rebuilding travis again, might just be flaky tests.

The apache rate limit problem will come back though. Let me see if I can find the line from previous travis builds that led me to say the build on travis is (for mysterious reasons) trying to use solr 6.5.0.... alas, it seems to be gone from history, perhaps when you travis rebuild (rather than a new build) it doesn't keep the log from the previous run of that build. Well, we'll see if it comes back.

And I'll try a simple travis rebuild again to see if it goes green this time... or gets the apache rate limit again.

RudyOnRails commented 6 years ago

@jrochkind yeah, I peeked at that failing build and there was only 1 failure that had to do with a click event that was being called on something that was nil/notfound. If that pops up, we can look into inserting a sleep or something.

jrochkind commented 6 years ago

Okay, with the release of hydra-derivatives 3.4.1, sufia is now green on master, without this PR.

So we could just leave it be.

Any thoughts on any of this? @mjgiarlo @RudyOnRails ? Anyone else anyone knows using sufia 7?

jrochkind commented 6 years ago

Okay, since the build is passing on master now, without this PR, I'm just going to close this one.

Not sure why the build is being done with sufia 6 and not 7, it wouldn't pass on sufia 7, but good enough for now.