Issues API unit tests #116

BenFradet · 2017-05-04T18:07:04Z

Still need to add test cases to ApiSpec

BenFradet · 2017-05-04T19:35:29Z

The command "sbt ++$TRAVIS_SCALA_VERSION test:fastOptJS" exited with 126.
0.00s$ sbt ++$TRAVIS_SCALA_VERSION orgScriptCI
/home/travis/.travis/job_stages: line 724: /usr/local/bin/sbt: Permission denied

fedefernandez · 2017-05-04T19:55:00Z

I'm not sure what could be the cause. I'll try to take a look tomorrow.

juanpedromoreno · 2017-05-04T20:31:04Z

@BenFradet @fedefernandez This might solve the issues: #117

codecov-io · 2017-05-04T21:15:12Z

Codecov Report

Merging #116 into master will increase coverage by 0.77%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
+ Coverage    87.4%   88.17%   +0.77%     
==========================================
  Files          36       36              
  Lines         516      516              
  Branches        3        3              
==========================================
+ Hits          451      455       +4     
+ Misses         65       61       -4

Impacted Files	Coverage Δ
...4s/shared/src/main/scala/github4s/GithubAPIs.scala	`100% <0%> (+3.22%)`	⬆️
...4s/shared/src/main/scala/github4s/api/Issues.scala	`100% <0%> (+30%)`	⬆️

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 c38ee4d...b09a98f. Read the comment docs.

BenFradet · 2017-05-05T21:21:35Z

I still have a few unit tests failing, still investigating why

BenFradet · 2017-05-06T10:05:45Z

github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala

+      request
+        .withMethod("GET")
+        .withPath(s"/search/issues")
+        .withQueryStringParameters(new Parameter("q", s"$nonExistentSearchQuery+"))


have you guys tried withQueryStringParameters before?
Never seems to match.

fixed it with regex 👍

BenFradet · 2017-05-06T10:07:37Z

github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala

+    response should be('left)
+  }
+
+  "Issues >> Edit" should "return the edited issue if valid data is provided" ignore {


for some reason this one's failing too

You guys seem to have used post for patch request for the git data API: https://github.com/47deg/github4s/blob/master/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala#L328-L348.

So I'm assuming there is something wrong with how mock server deals with patch?

We should create an issue to fix that part. The root of the problem is the call to postData. In that part the request already has set the PATCH method but if we take a look to this method in scalaj-http we can see that it's setting the method to POST.

One solution could be to replace the postData call by:

-- .postData(body) ++ .postData(d).method(rb.httpVerb.verb)

But I've not tested it yet. Sorry about the confusion

I get it, thanks. I'll log an issue 👍

BenFradet · 2017-05-06T16:56:22Z

Good to review 👍

fedefernandez

This is great, thanks for your effort. I've left some minor comments, I don't understand why the tests in ApiSpec are passing so I appreciate some clarifications around that. Other than that, looks good to me!

fedefernandez · 2017-05-06T17:38:41Z

github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala

+    .when(
+      request
+        .withMethod("POST")
+        .withPath(s"/repos/$validRepoOwner/$validRepoName/issues")


Should it be invalidRepoName?

true, I don't know how the tests manage to pass 😕

fedefernandez · 2017-05-06T17:39:29Z

github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala

+  mockServer
+    .when(
+      request
+        .withMethod("PATCH")


I think that our http client is applying a POST method for this operation

Yup I modified those, the sync doesn't seem to be reflected on gh, weird as well.

fedefernandez · 2017-05-06T17:39:56Z

github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala

+      request
+        .withMethod("GET")
+        .withPath(s"/search/issues")
+        .withQueryStringParameters(new Parameter("q", s"$nonExistentSearchQuery+"))


BenFradet

I don't know what's going on with GH but the sync isn't reflected here.

BenFradet · 2017-05-06T17:54:48Z

github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala

+    .when(
+      request
+        .withMethod("POST")
+        .withPath(s"/repos/$validRepoOwner/$validRepoName/issues")


true, I don't know how the tests manage to pass 😕

BenFradet · 2017-05-06T17:55:47Z

github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala

+  mockServer
+    .when(
+      request
+        .withMethod("PATCH")


Yup I modified those, the sync doesn't seem to be reflected on gh, weird as well.

BenFradet · 2017-05-06T17:57:47Z

nvm, pushed to my remote :'(

BenFradet · 2017-05-06T18:18:15Z

The command "sbt ++$TRAVIS_SCALA_VERSION orgScriptCI" exited with 137.

on 2.12

fedefernandez · 2017-05-06T18:26:41Z

/home/travis/.travis/job_stages: line 738: 5784 Killed
😞 retry?

Issues API unit tests

4414861

Tries to fix travis build issues (#117)

5af2217

BenFradet commented May 6, 2017

View reviewed changes

BenFradet changed the title ~~[wip] Issues API unit tests~~ Issues API unit tests May 6, 2017

fedefernandez approved these changes May 6, 2017

View reviewed changes

BenFradet commented May 6, 2017

View reviewed changes

ApiSpec unit tests for the Issues API

b09a98f

BenFradet force-pushed the issue_tests branch from 49f2463 to b09a98f Compare May 6, 2017 17:58

juanpedromoreno approved these changes May 6, 2017

View reviewed changes

juanpedromoreno merged commit af46078 into master May 8, 2017

juanpedromoreno deleted the issue_tests branch May 8, 2017 07:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issues API unit tests #116

Issues API unit tests #116

BenFradet commented May 4, 2017

BenFradet commented May 4, 2017

fedefernandez commented May 4, 2017

juanpedromoreno commented May 4, 2017

codecov-io commented May 4, 2017 •

edited

Loading

BenFradet commented May 5, 2017

BenFradet May 6, 2017

BenFradet May 6, 2017

fedefernandez May 6, 2017

BenFradet May 6, 2017

BenFradet May 6, 2017

fedefernandez May 6, 2017 •

edited

Loading

BenFradet May 6, 2017

BenFradet commented May 6, 2017

fedefernandez left a comment

fedefernandez May 6, 2017

BenFradet May 6, 2017

fedefernandez May 6, 2017

BenFradet May 6, 2017

fedefernandez May 6, 2017

BenFradet left a comment

BenFradet May 6, 2017

BenFradet May 6, 2017

BenFradet commented May 6, 2017

BenFradet commented May 6, 2017

fedefernandez commented May 6, 2017

Issues API unit tests #116

Issues API unit tests #116

Conversation

BenFradet commented May 4, 2017

BenFradet commented May 4, 2017

fedefernandez commented May 4, 2017

juanpedromoreno commented May 4, 2017

codecov-io commented May 4, 2017 • edited Loading

Codecov Report

BenFradet commented May 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedefernandez May 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenFradet commented May 6, 2017

fedefernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenFradet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenFradet commented May 6, 2017

BenFradet commented May 6, 2017

fedefernandez commented May 6, 2017

codecov-io commented May 4, 2017 •

edited

Loading

fedefernandez May 6, 2017 •

edited

Loading