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.

Sign up for GitHub

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

Jump to bottom

Test member data #3

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Test member data #3

wants to merge 6 commits into from

Conversation

ondenman
Copy link
Contributor

@ondenman ondenman commented Apr 27, 2017 •
edited
Loading

Tests scraper output against YAML file.
MemberPage#to_h should match the data specified in the YAML.

Part of everypolitician/everypolitician#600 (Ensure every scraper that has been ported to Scraped has a basic YAML test for at least one person)

Note: The MemberLink class is not scraping data correctly. The image field is populated with the page url and all other fields are empty. I've left a note in the test data and have opened an issue: #2

Oliver Denman added 4 commits April 27, 2017 15:01
@ondenman ondenman force-pushed the test-member-data--refactor branch from 5e70915 to d887903 Compare April 27, 2017 14:40
@ondenman ondenman changed the title Test member data refactor Test member data Apr 28, 2017
@ondenman ondenman force-pushed the test-member-data--refactor branch from d887903 to a737aee Compare April 28, 2017 09:12
@ondenman ondenman requested a review from tmtmtmtm April 28, 2017 13:14
tmtmtmtm
tmtmtmtm suggested changes May 1, 2017
View reviewed changes
Copy link
Contributor

@tmtmtmtm tmtmtmtm left a comment

Choose a reason for hiding this comment

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

There isn't really much value in adding tests like these when the scraper is so badly broken. It would be much better to add a real data test, and then make it work.

@tmtmtmtm tmtmtmtm assigned ondenman and unassigned tmtmtmtm May 1, 2017
@ondenman
Copy link
Contributor Author

It appears the scraper is pulling in data correctly. The way I'm generating the tests is broken. I'm going to replace the tests 3a67307 and a737aee

@ondenman
Copy link
Contributor Author

…of course, the target class is a fragment. What I really need to test is a subset of MembersPage.

@ondenman
Copy link
Contributor Author

ondenman commented Jul 7, 2017

I've opened a separate PR for the first three commits. #4

@ondenman ondenman mentioned this pull request Jul 7, 2017
Merged
@ondenman ondenman assigned tmtmtmtm and unassigned ondenman Jul 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmtmtmtm tmtmtmtm tmtmtmtm requested changes

Assignees

@tmtmtmtm tmtmtmtm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ondenman @tmtmtmtm