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

Fix Gadget API-Do not echo stuff to wikipedia #251

Merged
merged 32 commits into from Oct 31, 2017
Merged

Fix Gadget API-Do not echo stuff to wikipedia #251

merged 32 commits into from Oct 31, 2017

Conversation

GlazerMann
Copy link
Collaborator

@GlazerMann GlazerMann commented Oct 20, 2017

Fixes Gadget API.

Bot sends this when processes page with simply "MyText":
[15:36:43] Processing page ...
{"expandedtext":"MyText","editsummary":"/* Testing */ | [[WP:UCB|Assisted by Citation bot]]"}
Leads to error on wiki side because of processing page line is not valid json:
SyntaxError: JSON.parse: expected ',' or ']' after array element at line 2 column 4 of the JSON data

I was confused about cause. Turns out that HTML had nothing to do with it.

This turns on the eating of the buffer sooner.

It appears that [15:36:43] Processing page ... is echoed in the GadgetAPI and that is bad
@codecov
Copy link

codecov bot commented Oct 20, 2017

Codecov Report

Merging #251 into development will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                @@
##             development     #251      +/-   ##
=================================================
- Coverage          73.52%   73.47%   -0.05%     
+ Complexity          1236     1234       -2     
=================================================
  Files                  8        8              
  Lines               2338     2334       -4     
=================================================
- Hits                1719     1715       -4     
  Misses               619      619
Impacted Files Coverage Δ Complexity Δ
Page.php 40.32% <ø> (-1.26%) 54 <0> (-2)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06782d2...e9368eb. Read the comment docs.

@GlazerMann GlazerMann changed the title add $echo_html option add $echo_html option-Fix Gadget API Oct 20, 2017
@GlazerMann GlazerMann changed the title add $echo_html option-Fix Gadget API Fix Gadget API-Do not echo stuff to wikipedia Oct 21, 2017
@ms609
Copy link
Owner

ms609 commented Oct 24, 2017

This brings to my attention the lack of any test cases for gadgetapi.php ... at a minimum, is it possible to create a case (probably in a new file gadgetapiTest.php in the testcases folder) that checks that the output of the page is valid JSON in a couple of instances?

@GlazerMann
Copy link
Collaborator Author

is it possible to create a case that checks that the output of the page is valid JSON in a couple of instances.

I do not think that this is straight forward. We would need to:

  1. setup PHP's webserver to run the gadgetapi.php page. See: http://symfony.com/doc/current/setup/built_in_web_server.html
  2. query this webserver with an http request and save the output
  3. parse this data as json and verify it

@@ -564,6 +564,30 @@ public function testConvertJournalToBook() {
$this->assertEquals('cite book', $expanded->wikiname());
}

public function testGadgetAPI() {
Copy link
Owner

Choose a reason for hiding this comment

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

What we need to test, if anything, is that nothing is echoed when gadgetapi.php is run (except the final output). As you say, the functionality itself is tested in TemplateTest.php and elsewhere. It's not clear that this tests that.

Honestly, all we are checking is that the json encoder in PHP works.  If PHP is that broken, then nothing is trustworthy, including our tests.
@codecov-io
Copy link

codecov-io commented Oct 31, 2017

Codecov Report

Merging #251 into development will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##             development    #251      +/-   ##
================================================
- Coverage          73.52%   73.5%   -0.03%     
+ Complexity          1236    1235       -1     
================================================
  Files                  8       8              
  Lines               2338    2336       -2     
================================================
- Hits                1719    1717       -2     
  Misses               619     619
Impacted Files Coverage Δ Complexity Δ
Page.php 40.32% <ø> (-1.26%) 54 <0> (-2)
Template.php 81.46% <0%> (+0.02%) 1167% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06782d2...83993e6. Read the comment docs.

@ms609 ms609 merged commit b6a6476 into ms609:development Oct 31, 2017
@GlazerMann GlazerMann deleted the patch-1 branch October 31, 2017 15:22
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.

None yet

3 participants