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

Inconsistent, non-OO or non-declarative tests should be improved #48

Closed
victornoel opened this issue May 8, 2018 · 15 comments
Closed
Labels
bug Something isn't working

Comments

@victornoel
Copy link
Contributor

I noticed during my last PR that the quality standards of cactoos-http's tests are not at their best.

In particular we have many different ways of asserting facts depending on the test class:

  • Matchers.equalTo() or new EqualTo() to check equality while most of the time InputHasContent or TextHasString would be perfect (see also Matchers that take String should also take Text cactoos#836).
  • assertions on Map.get while Matchers.hasEntry (or its OO equivalent IsMapContaining) could make things more declarative.
  • calls to Scalar.value() while SclaraHasValue could be used.
  • we should prefer using the Matcher's class constructor instead of the static factory method from Matchers. For example use new StringEndsWith() instead of Matchers.endsWith().

I propose to fix this as soon as possible before cactoos-http becomes bigger and it becomes cumbersome to fix all those problems.

@0crat
Copy link
Collaborator

0crat commented May 8, 2018

@llorllale/z please, pay attention to this issue

@llorllale
Copy link
Collaborator

@victornoel let's follow the same path as in yegor256/cactoos#588

@llorllale
Copy link
Collaborator

@0crat in

@llorllale llorllale added the bug Something isn't working label May 15, 2018
@0crat 0crat added the scope label May 15, 2018
@0crat
Copy link
Collaborator

0crat commented May 15, 2018

@0crat in (here)

@llorllale Job #48 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented May 15, 2018

Bug was reported, see §29: +15 point(s) just awarded to @victornoel/z

@paulodamaso
Copy link
Contributor

@llorllale I'd like to do this.

@0crat
Copy link
Collaborator

0crat commented May 16, 2018

The job #48 assigned to @Vatavuk/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this

@Vatavuk
Copy link
Contributor

Vatavuk commented May 17, 2018

@0crat wait until llorllale/cactoos-matchers#9 (comment) is merged

@0crat
Copy link
Collaborator

0crat commented May 17, 2018

@0crat wait until llorllale/cactoos-matchers#9 (comment) is ... (here)

@Vatavuk The impediment for #48 was registered successfully by @Vatavuk/z

@Vatavuk
Copy link
Contributor

Vatavuk commented Jun 23, 2018

@victornoel please close this issue. It was solved in #58

@0crat
Copy link
Collaborator

0crat commented Jun 23, 2018

@elenavolokhova/z please review this job completed by @Vatavuk/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat
Copy link
Collaborator

0crat commented Jun 23, 2018

The job #48 is now out of scope

@0crat 0crat removed the scope label Jun 23, 2018
@elenavolokhova
Copy link

@0crat quality good

@0crat
Copy link
Collaborator

0crat commented Jun 23, 2018

Order was finished, quality is "good": +35 point(s) just awarded to @Vatavuk/z

@0crat
Copy link
Collaborator

0crat commented Jun 23, 2018

Quality review completed: +8 point(s) just awarded to @elenavolokhova/z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants