Fix URLs for multiple media in html_for_tweet()

philgyford commented

2017-10-07 10:52:33 -07:00

(Migrated from github.com)

I moved the html_for_tweet() tests into their own file.
Then fixed an issue with html_for_tweet() displaying media links when there are multiple media items.

As an example, one tweet that had multiple images was having its URL(s) come out something like:

wNc6uJklg" rel="external">pic.twitter.com/OwNc6uJklg</a>wNc6uJklg" rel="external">pic.twitter.com/OwNc6uJklg</a>

rather than

<a href="https://t.co/OwNc6uJklg" class="twython-media">pic.twitter.com/OwNc6uJklg</a>

* I moved the `html_for_tweet()` tests into their own file. * Then fixed an issue with `html_for_tweet()` displaying media links when there are multiple media items. As an example, one tweet that had multiple images was having its URL(s) come out something like: > `wNc6uJklg" rel="external">pic.twitter.com/OwNc6uJklg</a>wNc6uJklg" rel="external">pic.twitter.com/OwNc6uJklg</a>` rather than > `<a href="https://t.co/OwNc6uJklg" class="twython-media">pic.twitter.com/OwNc6uJklg</a>`

coveralls commented

2017-10-07 10:54:27 -07:00

(Migrated from github.com)

Coverage decreased (-0.7%) to 56.498% when pulling a27efd9da8 on philgyford:fix-media-in-html-for-tweet into 12e6b34d1d on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/13616507/badge)](https://coveralls.io/builds/13616507) Coverage decreased (-0.7%) to 56.498% when pulling **a27efd9da8b6eb7350d650c0fc14aca80c32b466 on philgyford:fix-media-in-html-for-tweet** into **12e6b34d1df5f292c6d867d2358d43d0e7c40142 on ryanmcgrath:master**.

philgyford commented

2017-10-07 13:41:30 -07:00

(Migrated from github.com)

Hmm, damn, not sure what I've done to reduce coverage. I can't work it out from coveralls.

michaelhelmick commented

2017-10-09 09:05:12 -07:00

(Migrated from github.com)

@philgyford looks like api.py increased by 4 lines.

Yours: https://coveralls.io/builds/13616507/source?filename=twython%2Fapi.py
Master: https://coveralls.io/builds/13633835/source?filename=twython%2Fapi.py

You can see lines that are hit and aren't. It shouldn't be too big of a change though. Looks like nothing actually is missed.

@philgyford looks like api.py increased by 4 lines. Yours: https://coveralls.io/builds/13616507/source?filename=twython%2Fapi.py Master: https://coveralls.io/builds/13633835/source?filename=twython%2Fapi.py You can see lines that are hit and aren't. It shouldn't be too big of a change though. Looks like nothing actually is missed.

michaelhelmick commented

2017-10-09 09:05:41 -07:00

(Migrated from github.com)

It looks like Python 2.x fails on Travis https://travis-ci.org/ryanmcgrath/twython/builds/284748942?utm_source=github_status&utm_medium=notification

philgyford commented

2017-10-09 09:35:12 -07:00

(Migrated from github.com)

Whoops, I'll fix it later today/tomorrow. Getting so used to python 3 these days I forget about 2!

coveralls commented

2017-10-10 04:44:03 -07:00

(Migrated from github.com)

Coverage increased (+0.4%) to 57.568% when pulling 5c55aa8844 on philgyford:fix-media-in-html-for-tweet into 12e6b34d1d on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/13647574/badge)](https://coveralls.io/builds/13647574) Coverage increased (+0.4%) to 57.568% when pulling **5c55aa88449a7110da1e1c4fea130d9109f303a6 on philgyford:fix-media-in-html-for-tweet** into **12e6b34d1df5f292c6d867d2358d43d0e7c40142 on ryanmcgrath:master**.

philgyford commented

2017-10-10 04:45:41 -07:00

(Migrated from github.com)

I've:

Fixed the python 2 issue
Added another couple of html_for_tweet() tests to help bump the coverage back up a bit
Got rid of some un-needed data from some of the test tweet objects

I've: * Fixed the python 2 issue * Added another couple of `html_for_tweet()` tests to help bump the coverage back up a bit * Got rid of some un-needed data from some of the test tweet objects

philgyford commented

2017-10-10 04:47:31 -07:00

(Migrated from github.com)

BTW, I was thinking it would make things a bit easier to follow if the test tweets were nicely-formatted JSON, that was loaded in to use in tests, rather than already being compact python dicts. I was just wary of messing around with things too much. Sound a good idea?

michaelhelmick commented

2017-10-11 04:50:41 -07:00

(Migrated from github.com)

@philgyford I think a python file with dicts (that are formatted nicely) or a JSON file would be nice. Either or would achieve the "same" knowledge to edit. config.py looks like it just has a bunch of one line dicts :(

@philgyford I think a python file with dicts (that are formatted nicely) or a JSON file would be nice. Either or would achieve the "_same_" knowledge to edit. `config.py` looks like it just has a bunch of one line dicts :(

philgyford commented

2017-10-11 10:37:10 -07:00

(Migrated from github.com)

Whoops... do you have any idea how I'd reference a file like tests/tweets/basic.json from within a test, in a way that Travis etc will find it? That's worked for me before, but not this time!

Whoops... do you have any idea how I'd reference a file like `tests/tweets/basic.json` from within a test, in a way that Travis etc will find it? That's worked for me before, but not this time!

coveralls commented

2017-10-11 10:43:01 -07:00

(Migrated from github.com)

Coverage increased (+0.4%) to 57.568% when pulling d3f5361f4d on philgyford:fix-media-in-html-for-tweet into 12e6b34d1d on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/13673500/badge)](https://coveralls.io/builds/13673500) Coverage increased (+0.4%) to 57.568% when pulling **d3f5361f4d58b693f013eeb07dd12b698cae5a58 on philgyford:fix-media-in-html-for-tweet** into **12e6b34d1df5f292c6d867d2358d43d0e7c40142 on ryanmcgrath:master**.

philgyford commented

2017-10-11 10:44:46 -07:00

(Migrated from github.com)

Phew, got it :) I think that's me done for now.

Rows
Columns

Fix URLs for multiple media in html_for_tweet() #461