Support collection types as a top level object

batuhansk commented 2 years ago

As you know, top level collection objects are also valid JSON. But, Mocker wasn't support them because of the casting that made up in the MockingURLProtocol.swift

batuhansk commented 2 years ago

refers to this issue #119

AvdLee commented 2 years ago

@batuhansk could you pull latest master into your PR so CI succeeds? Thanks!

batuhansk commented 2 years ago

@batuhansk could you pull latest master into your PR so CI succeeds? Thanks!

sure, done!

wetransferplatform commented 2 years ago

	Messages
:book:	View more details on Bitrise
:book:	MockerTests: Executed 26 tests (0 failed, 0 retried, 0 skipped) in 1.610 seconds

SwiftLint found issues

Severity	File	Reason
Error	MockingURLProtocol.swift:41	Line should be 160 characters or less: currently 166 characters (`line_length`)
Error	MockingURLProtocol.swift:44	Line should be 160 characters or less: currently 200 characters (`line_length`)
Warning	MockingURLProtocol.swift:101	Line should be 140 characters or less: currently 147 characters (`line_length`)
Error	MockingURLProtocol.swift:105	Line should be 160 characters or less: currently 221 characters (`line_length`)
Error	Mock.swift:8	Line should be 160 characters or less: currently 177 characters (`line_length`)
Warning	Mock.swift:49	Line should be 140 characters or less: currently 155 characters (`line_length`)
Error	Mock.swift:81	Line should be 160 characters or less: currently 211 characters (`line_length`)
Error	Mock.swift:84	Line should be 160 characters or less: currently 208 characters (`line_length`)
Error	Mock.swift:93	Line should be 160 characters or less: currently 287 characters (`line_length`)
Warning	Mock.swift:122	Line should be 140 characters or less: currently 142 characters (`line_length`)
Warning	Mock.swift:129	Line should be 140 characters or less: currently 141 characters (`line_length`)
Error	Mock.swift:136	Line should be 160 characters or less: currently 246 characters (`line_length`)
Error	Mock.swift:137	Line should be 160 characters or less: currently 236 characters (`line_length`)
Warning	Mock.swift:148	Line should be 140 characters or less: currently 150 characters (`line_length`)
Warning	Mock.swift:149	Line should be 140 characters or less: currently 153 characters (`line_length`)
Warning	Mock.swift:185	Line should be 140 characters or less: currently 159 characters (`line_length`)
Error	Mock.swift:195	Line should be 160 characters or less: currently 168 characters (`line_length`)
Warning	MockerTests.swift:163	Line should be 140 characters or less: currently 151 characters (`line_length`)
Warning	MockerTests.swift:181	Line should be 140 characters or less: currently 149 characters (`line_length`)
Warning	MockerTests.swift:239	Line should be 140 characters or less: currently 154 characters (`line_length`)
Warning	MockerTests.swift:52	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:71	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:94	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:116	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:149	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:165	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:183	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:206	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:259	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:457	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:491	Chained function calls should be either on the same line, or one per line. (`multiline_function_chains`)
Warning	MockerTests.swift:334	Lines should not have trailing whitespace. (`trailing_whitespace`)

Code Coverage Report

Name	Coverage
Mocker.framework	87.93%	✅

Generated by :no_entry_sign: Danger Swift against accd9f54ae6658f11835a7773b9e40c4ad085701

wetransferplatform commented 2 years ago

Congratulations! :tada: This was released as part of Release 2.7.0 :rocket:

Generated by GitBuddy

AvdLee commented 2 years ago

This PR made release v2.7.0 break the contract (APIs), thus who updated from 2.6.0, this change broke their envs. Was this expected? Shouldn't this have been a major version bump (e.g. v3.0.0)? 😬 @batuhansk @AvdLee

Ah, you're right! That went unnoticed, mostly since I think I thought Any would allow existing code to continue. We've didn't notice much issues on our side at least.

Did you run into many build errors?

rogerluan commented 2 years ago

Yes, our tests started failing upon upgrading from v2.6.0 to v2.7.0, so for now we locked on v2.6.0 (.exact("2.6.0")) until we (you and I?) figure out the versioning of Mocker... I think there are a few options:

a. Revert this PR, and re-release v2.7.0 without it (force push tags and so on - this requires rewriting git history). Then release this PR as part of v3.0.0 (since it includes a breaking change). Down side of this is that whoever has already updated their tests to the new interface will have to undo.
b. Revert this PR, and release v2.8.0. This doesn't require rewriting git history. There is no down side to this, except who decides to upgrade to the latest version again (v2.8.0) will have to revert their interface changes.
c. Keep everything as is and force everyone to update their code if they wanna use v2.7.0.
d. Create workarounds/adapt this PR to be backwards compatible in its interface. This requires re-thinking the approach of this PR, thus it's the highest-effort solution IMO. Might not even be possible without changing the interface. Once we figure out if this is possible, implement it and release as either v2.7.0 (force push), or a new v2.8.0.

Whatever way we decide, we have to inform users how to migrate from one to another I think (regardless if they're doing this in v2.7.0, v2.8.0 or v3.0.0). This can be done via a e.g. MIGRATION.md or just release notes.

WDYT @AvdLee ?

AvdLee commented 2 years ago

Could you show me the errors you got? Would be great if we can open a branch with that error reproduced. That way, I could play around with the API and make the code work for both backwards compatibility, as well for @batuhansk's case

rogerluan commented 2 years ago

I don't have the build logs anymore and would take some time to reproduce the environment in which this occurs, but the error was here:

mock.onRequest = { _, body in
    guard let body = body else { return }
    XCTAssertEqual(body["answers"] as? [String], answers)
    parametersExpectation.fulfill()
}

body in v2.6.0 is a [String:Any] whereas in v2.7.0 it's Any. Perhaps if I blindly casted it to [String:Any] it would work, but I didn't check it back then and decided to go for the safest route first.

So the error was like "body of type Any doesn't have a subscript etc…" something along those lines :)

Does this help @AvdLee ? 🙇

AvdLee commented 2 years ago

@rogerluan makes sense! I've opened an issue here: https://github.com/WeTransfer/Mocker/issues/132

I can't promise any timelines here, but feel free to jump on it if you have the time!

wetransferplatform commented 2 years ago

Congratulations! :tada: This was released as part of Release 3.0.0 :rocket:

Generated by GitBuddy

WeTransfer / Mocker

Support collection types as a top level object #125

SwiftLint found issues

Code Coverage Report