Skip to content

CFC: Feature/mockk integration - #188

Open
rybalkinsd wants to merge 2 commits into
masterfrom
feature/mockk-integration
Open

CFC: Feature/mockk integration#188
rybalkinsd wants to merge 2 commits into
masterfrom
feature/mockk-integration

Conversation

@rybalkinsd

@rybalkinsd rybalkinsd commented May 14, 2020

Copy link
Copy Markdown
Owner
    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

rybalkinsd commented May 14, 2020

Copy link
Copy Markdown
Owner Author

cc @neisip

@codecov

codecov Bot commented May 14, 2020

Copy link
Copy Markdown

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()
} No newline at end of file

Copy link
Copy Markdown
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
Copy Markdown
Owner Author

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

@rybalkinsd

Copy link
Copy Markdown
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

IVSivak commented May 19, 2020

Copy link
Copy Markdown
Collaborator

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

@rybalkinsd
rybalkinsd force-pushed the feature/mockk-integration branch from 67dbe86 to 7bd0ac7 Compare May 24, 2020 00:10
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