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

CFC: Feature/mockk integration #188

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rybalkinsd
Copy link
Owner

@rybalkinsd rybalkinsd commented May 14, 2020

    every {
        httpPost(init = matching {
            url("https://github.com")
            body {
                json {
                    "abc" to 123
                }
            }
        })
    } returns Response {
        request(mockk())
        protocol(mockk())
        code(101)
        message("...")
    }

@rybalkinsd rybalkinsd requested a review from IVSivak May 14, 2020 11:36
@rybalkinsd rybalkinsd self-assigned this May 14, 2020
@rybalkinsd rybalkinsd linked an issue May 14, 2020 that may be closed by this pull request
@rybalkinsd
Copy link
Owner Author

rybalkinsd commented May 14, 2020

cc @neisip

@codecov
Copy link

codecov bot commented May 14, 2020

Codecov Report

Merging #188 into master will decrease coverage by 5.49%.
The diff coverage is 27.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #188      +/-   ##
============================================
- Coverage     90.59%   85.10%   -5.50%     
- Complexity      127      131       +4     
============================================
  Files            42       44       +2     
  Lines           404      443      +39     
  Branches         49       63      +14     
============================================
+ Hits            366      377      +11     
- Misses           13       26      +13     
- Partials         25       40      +15     
Impacted Files Coverage Δ Complexity Δ
...ithub/rybalkinsd/kohttp/dsl/context/HttpContext.kt 66.12% <10.00%> (-24.57%) 15.00 <1.00> (+1.00) ⬇️
...hub/rybalkinsd/kohttp/dsl/context/HeaderContext.kt 63.63% <33.33%> (-36.37%) 5.00 <1.00> (+1.00) ⬇️
...thub/rybalkinsd/kohttp/dsl/context/ParamContext.kt 71.42% <33.33%> (-28.58%) 6.00 <1.00> (+1.00) ⬇️
...tlin/io/github/rybalkinsd/kohttp/mockk/Matchers.kt 57.14% <57.14%> (ø) 1.00 <1.00> (?)
...tlin/io/github/rybalkinsd/kohttp/mockk/Response.kt 100.00% <100.00%> (ø) 0.00 <0.00> (?)

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 ccf2994...7bd0ac7. Read the comment docs.

*/
fun Response(init: okhttp3.Response.Builder.() -> Unit): okhttp3.Response {
return okhttp3.Response.Builder().also(init).build()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

No newline

@rybalkinsd
Copy link
Owner Author

Codcov tests are expected to fail now. Will bring more when we will discuss the API

@rybalkinsd
Copy link
Owner Author

Moved further with mocks.
I found other possible declarations for returns statements

every {
    httpGet(init = matching {
       ...
    })
} returns ...

Option 2:

returns mockk {
    every { code() } returns 42
    every { message() } returns ""
}

Pros:

  • Looks like it's a more mockk-native way
  • Do not need to provide a mock object for request and protocol
  • Do not need to provide a DSL to declare Response objects

Cons:

  • Still rather verbose

Option 1 was:

returns Response {
    request(mockk())
    protocol(mockk())
    code(101)
    message("...")
}

Will be great to here your thoughts @IVSivak @gokulchandra @DeviantBadge

@IVSivak
Copy link
Collaborator

IVSivak commented May 19, 2020

The first option looks like more commonly used. The second option may be confusing for someone, but it looks concise.

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.

[kohttp-mockk] Mockk integration
2 participants