davidchambers / tutor

JavaScript interface for the Gatherer card database
https://gatherer.wizards.com/
Do What The F*ck You Want To Public License
149 stars 18 forks source link

Added check for multi-result pages to prevent null exception error. #96

Closed CBXZero closed 8 years ago

CBXZero commented 9 years ago

First time I attempted the squash, I followed a stackoverflow answer that was misleading. This should be good to go and is all bundled in 1 commit :panda_face:

davidchambers commented 9 years ago

Thank you for persevering despite your version control troubles. :)

Please git commit --amend to give the commit a sensible message. You'll then need to run git push origin master --force to update this pull request.

CBXZero commented 9 years ago

Updated the commit and pull request title to be informative :)

CBXZero commented 9 years ago

That "and" looked weird to me and the build test failed, if this build doesn't work I'll fix it when I can get to my laptop

davidchambers commented 9 years ago

Tests are failing. ;)

CBXZero commented 9 years ago

Looking at it on my dev machine now, I'll have it fixed here in a sec :) sorry for the flux of emails you must be getting. D:

CBXZero commented 9 years ago

Got it that time. I don't know why I thought that "and" looked funny. Then again, I've been studying Statistics all night and that might be messing with me. Lmk if there are any other changes you'd like done. :)

CBXZero commented 9 years ago

I did some looking into this, and 2 things stood out to me.

  1. I modified the javascript library by hand that my bot was using, and told it to just spit out the html of the file that it reads, and I noticed it hits the else block that is highlighted above multiple times (the file saves 3 times to be exact), and was wondering if you had any clues to that.
  2. The and is incorrect, it should be an || body.indexOf(string) >= 0 . That being said, if I actually switch it to that, I get 27 tests that break, mostly involving an Uncaught AssertionError: {} === null, which makes me think the double sided cards are now getting caught on that else block.

I was wondering if there were any ideas that you had that might be causing the issues I'm running into. I'll investigate this issue more tomorrow.

davidchambers commented 9 years ago

I suggest looking at the differences between the responses to the following requests:

$ curl --silent http://gatherer.wizards.com/Pages/Card/Details.aspx?name=fizzbuzzldspla
$ curl --silent http://gatherer.wizards.com/Pages/Card/Details.aspx?name=test
CBXZero commented 9 years ago

I'm still looking into this, just haven't been able to come back to it due to an interview and hw. I'll work on it more this next week.

davidchambers commented 8 years ago

Any luck so far, @CBXZero?

CBXZero commented 8 years ago

Taking a look at it today, was swamped with schoolwork for a while. I should have something by the end of this weekend.

CBXZero commented 8 years ago

Spent some time on this during the weekend, the results that I get from curling the URL and the body that I console log when testing are different and that's currently what I'm hung up on.

davidchambers commented 8 years ago

If you provide a reproducible example of the difference in behaviour between curl and node I will help you to investigate. :)

CBXZero commented 8 years ago

I console.logged the body on the line right before the callback around line 38, pasted it into a test editor, and compared it to the fixture generated and they were radically different. I think there is some recursive call that is throwing me off.

EDIT: I can send you the exact outputs tomorrow afternoon if you want.

CBXZero commented 8 years ago

Here are the bodies that get logged in order of when I console.log them (this is from a single tutor.card() call searching via name, in this case "test" was used):

https://gist.github.com/CBXZero/b9136fd563b7028e2165

https://gist.github.com/CBXZero/b3322224639489d4ce0b

https://gist.github.com/CBXZero/2e8ec5a0e98e844ac4ec

davidchambers commented 8 years ago

Superseded by #97