Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Unit tests for RtFavorites #41

Merged
merged 3 commits into from
Apr 28, 2017
Merged

Unit tests for RtFavorites #41

merged 3 commits into from
Apr 28, 2017

Conversation

SherifWaly
Copy link
Contributor

PR for #40

@coveralls
Copy link

coveralls commented Apr 27, 2017 •
edited

Coverage Status

Coverage increased (+20.2%) to 80.952% when pulling bd48b5e on SherifWaly:#40 into c6fda08 on decorators-squad:master.

@SherifWaly
Copy link
Contributor Author

@amihaiemil Please check this commit :)
I've added unit tests for RtFavorites should I add also integration tests? (query values would be different for me as I'm working on a fork)

@coveralls
Copy link

coveralls commented Apr 27, 2017 •
edited

Coverage Status

Coverage increased (+20.2%) to 80.952% when pulling 804dde9 on SherifWaly:#40 into c6fda08 on decorators-squad:master.

amihaiemil
amihaiemil reviewed Apr 28, 2017
View reviewed changes
Copy link
Member

@amihaiemil amihaiemil left a comment

Choose a reason for hiding this comment

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

@SherifWaly One comment. And yes, if you think you could do it fast (there are already several written), I would appreciate an integration test :D

If not, just leave a puzzle :D

src/test/java/com/amihaiemil/versioneye/RtFavoritesTestCase.java Outdated
MatcherAssert.assertThat(fetched.size(), Matchers.is(2));

MatcherAssert.assertThat(
fetched.get(0).name(), Matchers.equalTo("doctrine/common")
Copy link
Member

Choose a reason for hiding this comment

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

@SherifWaly Maybe you can take the Favorite in a value before, to avoid so many .get(0)? It would be more readable.

@amihaiemil
Copy link
Member

(query values would be different for me as I'm working on a fork)

What does this mean? What values, where?

@SherifWaly
Copy link
Contributor Author

@amihaiemil For RtUserITCase values returned are different as username is SherifWaly rather than amihaiemil. So for this class favorites will be also different so I just added a puzzle for it :)

@amihaiemil
Copy link
Member

@SherifWaly I see. Well yes, I still have to find a way of running those IT on Travis (they are not currently run there)... maybe we'll get a Test user :)

It's fine anyway, thanks

@coveralls
Copy link

Coverage Status

Coverage increased (+20.2%) to 80.952% when pulling 4489e6c on SherifWaly:#40 into c6fda08 on decorators-squad:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+20.2%) to 80.952% when pulling 4489e6c on SherifWaly:#40 into c6fda08 on decorators-squad:master.

@amihaiemil
Copy link
Member

@rultor merge please

@rultor
Copy link
Collaborator

rultor commented Apr 28, 2017

@rultor merge please

@amihaiemil OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 4489e6c into decorators-squad:master Apr 28, 2017
@rultor
Copy link
Collaborator

rultor commented Apr 28, 2017

@rultor merge please

@amihaiemil Done! FYI, the full log is here (took me 1min)

@SherifWaly SherifWaly deleted the #40 branch April 28, 2017 16:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@amihaiemil amihaiemil amihaiemil left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@SherifWaly @coveralls @amihaiemil @rultor @0pdd