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

Updated test suite #95

Merged
merged 3 commits into from
Jan 8, 2017
Merged

Updated test suite #95

merged 3 commits into from
Jan 8, 2017

Conversation

gRegorLove
Copy link
Member

@gRegorLove gRegorLove commented Apr 26, 2016

This updates the test suite to be included via Composer so we can more easily keep it up to date with https://github.com/microformats/tests, per #50.

The comparison of parsed mf2 to expected output now compares the complete array, not just the 'items' key. I took a stab at better array comparison, since the === comparison expects the array indexes to be in the same order.

Unfortunately these changes bring the number of passing tests to zero, but hopefully a lot of those are minor. For example, I know that php-mf2 puts the "T" separator in parsed dt- values, but the test suite expected output does not (see discussion), so most date values will have that discrepancy.

The updated array comparison should verify that every value in the expected output appears in the parsed output, but does not account for the parsed output having more than the expected output. The array comparison could be run twice (switching the array parameters) but I wasn't sure if that was desired. And I thought it was more important to make sure the parser returns at least the expected output.

@gRegorLove gRegorLove changed the title Updated test suite for #50 Updated test suite Apr 26, 2016
@gRegorLove
Copy link
Member Author

@aaronpk @dissolve @barnabywalters and anyone else: if you have time would you mind reviewing this PR? I'd like to get this set of three PRs merged so I can work on further improvements.

@aaronpk
Copy link
Member

aaronpk commented Jan 5, 2017

Can you commit the updated composer.lock file as well? Other than that, it looks good to me.

@gRegorLove
Copy link
Member Author

Ok, done.

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.

2 participants