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

rosie: restore rosie ws #2355

Merged
merged 9 commits into from
Jul 5, 2019
Merged

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Jun 13, 2019

NOTE: As sqlalchemy was installed to system Python rather than Travis Python some tests wern't being run (due to missing dependencies). These have been fixed.

  • Restore files removed in 4004944.
  • Re-jig .travis/now with a new RI_PATH for setting PATH in rose_site_init in env -i environments.
  • Fix tests.

@oliver-sanders oliver-sanders added this to the next-feature-alpha milestone Jun 13, 2019
@oliver-sanders oliver-sanders self-assigned this Jun 13, 2019
@oliver-sanders oliver-sanders requested a review from wxtim June 13, 2019 08:51
wxtim
wxtim previously approved these changes Jun 13, 2019
Copy link
Contributor

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

What is there looks ok. Is it sufficient?

@matthewrmshin
Copy link
Member

Travis CI build failing due to lack of request module?

@wxtim wxtim self-requested a review June 13, 2019 10:51
@oliver-sanders oliver-sanders force-pushed the restore-rosie-stuff branch 3 times, most recently from 571336e to dcbf8c0 Compare June 13, 2019 11:54
@oliver-sanders
Copy link
Member Author

oliver-sanders commented Jun 13, 2019

By adding the Rosie dependencies we are now activating tests which might not have been run since Python2.

Rosie tests are falling over in a peculiar way. It would appear svn_post_commit is being run under Python3.5 (OS provided) rather than 3.7 (Travis installed) where it then fails due to the missing sqlalchemy dependency which is installed in the 3.7 stack.

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Jun 13, 2019

In #2288 sqlalchemy was installed to system Python via apt-get.

  • Added rose_init_site updating PATH to include the dirs including python and rose in the event that $HOME is not defined (commit_hook scripts).

@oliver-sanders oliver-sanders force-pushed the restore-rosie-stuff branch 3 times, most recently from 3716468 to f42815d Compare June 13, 2019 14:37
@oliver-sanders oliver-sanders dismissed wxtim’s stale review June 13, 2019 14:37

not working yet :(

@oliver-sanders oliver-sanders force-pushed the restore-rosie-stuff branch 2 times, most recently from ff8b35a to e918e71 Compare June 13, 2019 15:25
@oliver-sanders oliver-sanders force-pushed the restore-rosie-stuff branch 2 times, most recently from 3e63550 to a2309eb Compare June 14, 2019 15:40
@oliver-sanders
Copy link
Member Author

The tests should pass this time:

@sadielbartholomew
Copy link
Contributor

I've skipped a failing test which is punted to #2357

I had a look-see at this, & it is failing for quite a significant reason: the rosie-disco web files are not there!

Because all other t/rosie-disco/ sub-tests were testing on ?format-json endpoints, the only two tests that were checking for rendered pages, which use the HTML templates & CSS & JS files from the lib/html/ directory in 2019.XX & previous versions, were the sub-tests for the root index page! And the second of those:

https://github.com/oliver-sanders/rose/blob/a2309eb25e6768d9c0628d9da75e1f3adb9f91b5/t/rosie-disco/00-basic.t#L65-L70

was just testing for a re-direct to the equivalent with the trailing slash, so it passed. But it redirected from a 500 Internal Sever Error, as was being returned for the redirected page causing that one sub-test failure.

Presumably at some point the web files required were removed from master. Though we no longer need the subset of them designed for Rose Bush, we still want them for the updated Python 3 Rosie Disco. I will put in a PR to this branch which re-adds the necessary directories (leaving out obvious Rose Bush files), & removes the comment characters on the currently-failing sub-test which should then pass (Travis, based on the local test battery pass I then see).

@oliver-sanders
Copy link
Member Author

Thanks Sadie, hopefully this should now pass.

@matthewrmshin
Copy link
Member

Travis CI unit test failing - need to filter out lib/html/ hierarchy?

@sadielbartholomew
Copy link
Contributor

Ah, & naturally those web files come a bundle of issues as flagged also by Codacy. Can we get them ignored & (re-)raised as an Issue? I can't recall quite how we dealt with those previously...

@matthewrmshin
Copy link
Member

On Codacy. We should filter out the external libraries from the analysis. Some of our own stuffs can be fixed. The others should be flagged for ignore.

@oliver-sanders oliver-sanders force-pushed the restore-rosie-stuff branch 2 times, most recently from 1525a2a to d558044 Compare June 18, 2019 11:07
@matthewrmshin
Copy link
Member

Branch in conflict.

@sadielbartholomew
Copy link
Contributor

@oliver-sanders what is the state of this PR, so I know when to review? Is the plan to address those Codacy points? Also I have kicked Travis CI but across a number of tries it keeps returning an error from a syntax error in .travis/now.

@wxtim wxtim mentioned this pull request Jul 5, 2019
20 tasks
@oliver-sanders
Copy link
Member Author

PR was still ready for review, tests broken by most recent conflict resolution.

@matthewrmshin
Copy link
Member

Unit test broken by style check?

@oliver-sanders
Copy link
Member Author

The three codacy issues can be ignored:

  • The Jinja2 autoescape comment is just wrong as this is set on the following line
  • The input comments are not relevant to Python3, Tim has ignored these issues in Another rose package branch #2360

@matthewrmshin
Copy link
Member

@wxtim Please perform a final sanity check. The Codacy/Bandit issue can be unflagged in your branch?

@wxtim wxtim merged commit 1d22304 into metomi:master Jul 5, 2019
@oliver-sanders oliver-sanders deleted the restore-rosie-stuff branch July 5, 2019 10:58
@sadielbartholomew sadielbartholomew removed their request for review October 25, 2019 14:46
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.

4 participants