Fixes #1412: Add titles to pages

[heatherbooker]

Fixes #1412 .

Also adds titles to all other possible pages (ie not humans.txt!)

[karlcow]

@heatherbooker I'm about to board a plane.
I will review as soon as I am on the other side. Thanks for the Pull request.

[heatherbooker]

ahah ok @karlcow thx for the heads up! ^^ safe trip!

[karlcow]

Thanks for the pull request.
Please make the changes and rebase it once done. :) If you don't know how to do it I will gladly explain.

[karlcow]

just putting this here, because I was experimenting in testing with the python side.

#!/usr/bin/env python
# -*- coding: utf-8 -*-
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

'''Tests for our URL endpoints.'''

import os.path
import sys
import unittest

# Add webcompat module to import path
sys.path.append(os.path.realpath(os.pardir))
import webcompat  # nopep8

# Any request that depends on parsing HTTP Headers (basically anything
# on the index route, will need to include the following: environ_base=headers
headers = {'HTTP_USER_AGENT': ('Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; '
                               'rv:31.0) Gecko/20100101 Firefox/31.0')}


class TestURLs(unittest.TestCase):
    def setUp(self):
        webcompat.app.config['TESTING'] = True
        self.app = webcompat.app.test_client()

    def tearDown(self):
        pass

    def test_home(self):
        '''Page title format for different URIs.'''
        website_uris = [
            ('/about', 'About'),
            ('/contributors', 'Contributors'),
            ('/tools/cssfixme', 'CSS Fix Me'),
            ('/issues/1000', 'Issue #1000'),
            ('/issues', 'Issues'),
            ('/issues/new', 'New Issue'),
            ('/privacy', 'Privacy Policy')]
        for uri, title in website_uris:
            rv = self.app.get(uri, environ_base=headers)
            expected = '<title>{title} | webcompat.com</title>'.format(
                title=title)
            self.assertTrue(expected in rv.data)


if __name__ == '__main__':
    unittest.main()

With the results:

nosetests -v tests/test_rendering.py
# Page title format for different URIs. ... ok
#
# ----------------------------------------------------------------------
# Ran 1 test in 0.181s
#
# OK

[karlcow]

My feeling on why it should be on the python side more than the JS side (functional tests) is that these titles do not require any actions from the user. It's really generated by the application. So it probably should be in the unit tests.

@heatherbooker What do you think about it?

[heatherbooker]

@karlcow Hm gotcha, I don't have a lot of experience testing but looking through the tests that does make sense to have it in unit tests.

Oh actually except for testing /activity/user - should that also be done from the python file?

And would you rather see these tests in a separate file test_rendering.py than included in test_urls.py? I just want to get your feedback so I can make the same sort of decision in the future - could you elaborate on why it doesn't belong in test_urls? Or calling it something like test_titles?

Last q - why the # nopep8 after import webcompat in the code you suggest? I'm just curious since I don't see it in the other files too!

[karlcow]

Sorry for the late answer

Oh actually except for testing /activity/user - should that also be done from the python file?

This should be done in python too, but is a bit more complicated. because it requires to mock the circumstances of the template.
https://github.com/heatherbooker/webcompat.com/blob/dde846243e783d5f883033f9d6303ee388fb4b6e/webcompat/views.py#L240

And would you rather see these tests in a separate file test_rendering.py than included in test_urls.py? I just want to get your feedback so I can make the same sort of decision in the future - could you elaborate on why it doesn't belong in test_urls? Or calling it something like test_titles?

Yup in a different file than test_urls.py
I don't have a strong opinion, but I have the feeling that it is a well defined scope that we could extend in the future. Such as testing also the content in the page.

Last q - why the # nopep8 after import webcompat in the code you suggest? I'm just curious since I don't see it in the other files too!

# nopep8 is a trick for avoiding the syntax linting to complain each time. For example, usually in python the import should be before the lines of code. But it happens sometimes that you need to put them after.

This is PEP8 E402. You can see the difference in between the lines here.

The why it's not everywhere is that we started to clean it up little by little when we are editing the files. But not necessary systematically for the whole project as it is not a bug, but more cosmetics.

Hope it helps.

[heatherbooker]

Hey @karlcow , my apologies for the delay! I've been fiddling with testing the activity/user route but still unsure how to mock the circumstances of the template.

I know you linked to the route in views.py and it has g.user and session['username'] but I haven't been able to figure it out! :( I don't see anything similar in the other python test files involving actually mocking being logged in. Here is the code. If you could point me in the right direction I would really appreciate it! :)

[karlcow]

@heatherbooker

Hey @karlcow , my apologies for the delay! I've been fiddling with testing the activity/user route but still unsure how to mock the circumstances of the template.

me too. I'm almost there but it's indeed a bit complex. What I propose is that you send the pull request without /activity/username test. And we open a subsequent new issue for this specific test.

In that way we do not block your contributions and we can address this specific test later on.

[heatherbooker]

Okie doke @karlcow , done :)

(Phew, it makes me feel better to know that you too find it a bit complex!)

[heatherbooker]

And here's a new issue for it - #1428

[karlcow]

Thanks a lot. Good work.