search

s-aska / p5-Plack-Session-State-URI

Other
1 stars 2 forks source link

2015 CPAN PR Challenge - PR #3 - Test HTML Parsing #6

Open bambams opened 9 years ago

bambams commented 9 years ago

This pull request introduces a new test program that demonstrates the flaws in the regular expression used to parse the HTML. I am new to writing unit tests, test-driven development, and writing tests in Perl too. This is just a contrived program to show the alleged flaws. I tried to test it by hard-coding a "correct" transformation of the input HTML and verifying that the tests all pass. Hopefully that is still true after rebasing.

The test program uses File::Temp to log diagnostic information that would be noisy on the terminal. It might be helpful to understand why the tests are failing. Additionally, it might be useful to identify bugs in the tests themselves once work begins on replacing the regular expression with a proper parser.

I originally wanted to submit another branch that actually fixes these tests by using HTML::Parser and reimplmenting both of HTML::FillInForms and HTML::StickyQuery. I ran out of time and motivation this month so officially that patch won't make it into the challenge, but I'll still do my best to come up with something when time and motivation permit unless you beat me to it. Make sure you don't release after merging this branch until fixing the HTML parsing because the tests will fail and users will be quite annoyed when make test fails! Fix the tests before submitting a new release to CPAN!

If you have any trouble merging feel free to contact me! I'll do the work of rebasing for you so you don't have to fuss with it! Thanks!

bambams commented 9 years ago

Heads up, I just rebased the branch on your latest master and rewrote the branch with push -f. If you do decide to merge that should make it easier for you.

bambams commented 9 years ago

Danger Will Robinson: I don't know why, but now the tests introduced by this pull request appear not to be failing properly. Only 1 test is failing, but all 10 of them should fail... I'm not seeing any updates from you to fix the regular expression so I'm at a loss for why the tests aren't failing now. To be fair, I am pretty drunk. ;)

I'll have to look into this to determine why it's not making sense... Perhaps tomorrow once I am rested and sober. In the meantime, this branch might be broken. I don't know if it was caused by the merge+rebase of my other branches or if I made a mistake with the tests... I'll update this thread when I have figured out what went wrong.

bambams commented 9 years ago

Well I figured out what was wrong... I caused a regression in your module! So embarrassing! See here for the fix: https://github.com/s-aska/p5-Plack-Session-State-URI/pull/8