search

ome / infrastructure

A repository containing scripts for managing infrastructure
BSD 2-Clause "Simplified" License
20 stars 19 forks source link

Reformat the tests, add more urls, add test for blog and non-suffix c… #295

Closed pwalczysko closed 7 years ago

pwalczysko commented 7 years ago

…ommunity urls

This is a follow up of https://github.com/openmicroscopy/infrastructure/pull/294#event-1171748453

To test, follow instruction in the header of https://github.com/openmicroscopy/infrastructure/pull/294#event-1171748453.

@sbesson : Added almost everything what I could from the whole g.doc but the biggest surprise seemed to be the failure of this test on the qa2 tab.

The only tab where I was not completely exhaustive was site/support as this seems to be a bit redundantly designed, and no errors were encountered on the legacy redirects there.

In summary:

were the only remaining questions here.

snoopycrimecop commented 7 years ago

Conflicting PR. Removed from build MANAGEMENT_TOOLS-merge#1495. See the console output for more details. Possible conflicts:

sbesson commented 7 years ago

Barring the conflict (try to merge origin/master into this branch maybe?), the refactoring and the new test coverage makes sense to me and matches our current knowledge.

Answering the specifics:

There are definitely known issues with the /Schemas URL which might require more work on the Apache configuration. Given the importance of this URL and our current deployment strategy, a safer proposal would be to card this limitation to investigate the migration of the schemas to the new ome-www boxes so that www.openmicroscopy.org/Schemas content can be served directly rather than proxying www-legacy /cc @kennethgillen

pwalczysko commented 7 years ago

Barring the conflict (try to merge

How does this conflict claimed by snoopy tie with the big green tick "This branch has no conflict ..." on this page

pwalczysko commented 7 years ago

@sbesson : So as expected, when I checked out new branch to track origin/master locally and did git fetch , afer which I merged this branch into it, all went smoothly. See below a start of git log on this merged branch. I have no conflicts to fix, and none are reported by snoopy either (Conflicts detected: None), see https://github.com/openmicroscopy/infrastructure/pull/295#issuecomment-316876360

commit 04b86662260c22f012ee235679c144840b9aab12
Author: Petr Walczysko <p.walczysko@dundee.ac.uk>
Date:   Thu Jul 20 19:07:28 2017 +0100

    Reformat the tests, add more urls, add test for blog and non-suffix community urls

commit 51de3debe2de79ecb0ac0b0ff2ca58c37a0cb6cb
Merge: 5a5bf70 88ae6e3
Author: Sébastien Besson <seb.besson@gmail.com>
Date:   Thu Jul 20 11:30:00 2017 +0200

    Merge pull request #294 from sbesson/redirects_test

    Redirects test extension
...
pwalczysko commented 7 years ago

@sbesson : Tried to merge origin/master to this branch directly.

$git checkout redirect-tests-follow-up 
Switched to branch 'redirect-tests-follow-up'
$ git merge origin/master
Already up-to-date.
$ git push pwalczysko HEAD 
Everything up-to-date
pwalczysko commented 7 years ago

@sbesson : Sorry, my bad. The qa2 links are just fine, added a commit checking them.

snoopycrimecop commented 7 years ago

Conflicting PR. Removed from build MANAGEMENT_TOOLS-merge#1496. See the console output for more details. Possible conflicts:

pwalczysko commented 7 years ago

@sbesson @manics @kennethgillen : I really do not understand why does snoopy report these non-existent conflicts (and is in discrepancy with the travis as well as my local merge tests and reports "None" as the list of conflicts)

manics commented 7 years ago

I can't see why it's failing either. I propose merging it once its tested and approved.

kennethgillen commented 7 years ago

From https://ci.openmicroscopy.org/job/MANAGEMENT_TOOLS-merge/1496/consoleText there's 2017-07-24 02:31:28,119 [ scc.git] INFO Conflict detection failed - presumably the fail is causing it to remove the PR. Not sure why it's failing though.

kennethgillen commented 7 years ago 
LS27172:www kenny$ py.test tests/check_redirects.py
========================================== test session starts ==========================================
platform darwin -- Python 2.7.13, pytest-3.1.3, py-1.4.34, pluggy-0.4.0
rootdir: /Users/kenny/Forks/infrastructure/ansible/server-state-playbooks/www, inifile:
plugins: pyfakefs-3.1
collected 105 items

tests/check_redirects.py .........................................................................................................

====================================== 105 passed in 6.22 seconds =======================================
LS27172:www kenny$
kennethgillen commented 7 years ago

@pwalczysko - let me know how you wish to proceed- I'm happy to merge as is, or you can add the s/ome-www/www to the PR and I'll also merge.

snoopycrimecop commented 7 years ago

Conflicting PR. Removed from build MANAGEMENT_TOOLS-merge#1497. See the console output for more details. Possible conflicts:

snoopycrimecop commented 7 years ago

Conflicting PR. Removed from build MANAGEMENT_TOOLS-merge#1498. See the console output for more details. Possible conflicts:

pwalczysko commented 7 years ago

This should fix @kennethgillen 's comments. Listed for tomorrow's review on Standup.

snoopycrimecop commented 7 years ago

Conflicting PR. Removed from build MANAGEMENT_TOOLS-merge#1499. See the console output for more details. Possible conflicts:

kennethgillen commented 7 years ago 
210 passed in 11.15 seconds

210 == 2 * 105, so looks good.

kennethgillen commented 7 years ago

👍