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

PsLinkedinTest.java:42-48: This class not implemented yet,... #158

Closed
davvd opened this issue Apr 8, 2015 · 23 comments
Closed

PsLinkedinTest.java:42-48: This class not implemented yet,... #158

davvd opened this issue Apr 8, 2015 · 23 comments
Milestone

Comments

@davvd
Copy link

davvd commented Apr 8, 2015

Puzzle 12-159d303e in src/test/java/org/takes/facets/auth/social/PsLinkedinTest.java:42-48 has to be resolved: This class not implemented yet, but has to be. Please implement it. Mocked OAuth server is one of the possible solutions. In this case, we could use some library like WireMock (see http://www.wiremock.org), which allows make stub for web service. We'll need to make some changes in PsLinkedin, that allow to change API url, and use local stubbed http server during unit test.... The puzzle was created by Dmitry Zaystev on 4-Apr-2015.

If you have any technical questions, don't ask me, submit new tickets instead

@ikhvostenkov
Copy link
Contributor

@davvd I can take this issue if no one does not mind

@yegor256
Copy link
Owner

@davvd pls assign @ikhvostenkov

@ikhvostenkov
Copy link
Contributor

@yegor256 I do not think this is good idea here to use WireMock or similar staff due to this will be not unit test but already integration test. I've looked other classes PsGithub and PsGoogle and there is the same method fetch in it, I suggest to extract this code in separate class and after that we can mock this class and after that very simple create tests for social auth. There is other way just to mock fetch method call inside class under test with NonStrictExpectations, but IMHO first approach is better due to we will also remove code duplication. How do you think?

@yegor256
Copy link
Owner

@ikhvostenkov well, the idea is good, but I doubt we can technically do this. all these fetch() methods are different in each Ps* class. I don't see how you can generalize them. But feel free to try, I like the idea.

@davvd davvd added this to the 1.0 milestone Apr 18, 2015
@davvd
Copy link
Author

davvd commented Apr 18, 2015

@yegor256 I set milestone here to 1.0, let me know if it is wrong

@davvd
Copy link
Author

davvd commented Apr 18, 2015

@celezar This task is yours, please go ahead keeping in mind this. If any questions, don't hesitate to ask right here... Task's budget is 30 mins (see this for explanation)

@davvd davvd added the @celezar label Apr 18, 2015
@davvd
Copy link
Author

davvd commented Apr 18, 2015

@davvd pls assign @ikhvostenkov

@yegor256 yes, done, @ikhvostenkov is at the task, please start! whoever worked with this task before, please stop immediately

@davvd
Copy link
Author

davvd commented Apr 18, 2015

@davvd pls assign @ikhvostenkov

@yegor256 sure! @ikhvostenkov the task is yours, please go ahead

@davvd
Copy link
Author

davvd commented Apr 18, 2015

@davvd pls assign @ikhvostenkov

@yegor256 done, @ikhvostenkov is a new assignment at the task, please go on. whoever worked with this task before, please stop right now

@yegor256
Copy link
Owner

@davvd looks like we need more time here

@davvd
Copy link
Author

davvd commented May 1, 2015

@davvd looks like we need more time here

@yegor256 right, take your time and thanks for informing

@davvd
Copy link
Author

davvd commented May 10, 2015

@yegor256 I am closing it, thanks all

@davvd davvd closed this as completed May 10, 2015
@davvd
Copy link
Author

davvd commented May 10, 2015

@elenavolokhova please, review this ticket for compliance with our QA rules

@elenavolokhova
Copy link

@ikhvostenkov
According to our quality rules:

Some of ticket messages must mention all pull requests where the problem was actually fixed.

There is no link to related PR in comments. Several PR's have links to this issue and it is not clear which one solves the problem. Please add this info and follow this rule in your tickets in the future.

@ikhvostenkov
Copy link
Contributor

@elenavolokhova PR which resolves this issue is not finished, I do not know why this ticket has been closed.
@davvd Why did you close this ticket?

@yegor256
Copy link
Owner

@ikhvostenkov the ticket was closed because the puzzle disappeared from the source code... make sense?

@ikhvostenkov
Copy link
Contributor

@yegor256 this make sense. But the question is why did puzzle disappear? Due to someone else resolve this? Why we have to identical tasks? What shall I do with this task then?

@yegor256
Copy link
Owner

@ikhvostenkov it happens sometimes, you just got lucky here, no need to do anything

@elenavolokhova
Copy link

@yegor256 So should I mark this issue as bad? Finally there is no solution provided.

@yegor256
Copy link
Owner

@elenavolokhova yeah, it's a good candidate for a "bad" mark

@elenavolokhova
Copy link

@davvd As per discussion above quality is bad.

@davvd
Copy link
Author

davvd commented May 14, 2015

@davvd As per discussion above quality is bad.

@elenavolokhova thanks for paying attention, the performer of this ticket won't be paid

@davvd
Copy link
Author

davvd commented May 14, 2015

@ikhvostenkov I added 10 mins to @elenavolokhova (for QA review) in transaction 57286878. I added 38 mins to your account, many thanks for working with the project! The completion time here was 57286885.. age=48 hours and 31 mins, that's why a bonus here for fast delivery. +38 to your rating, your total score is +77

@davvd davvd mentioned this issue Jun 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants