Add More Tests #21
Add More Tests #21
Conversation
|
Swift nightly tests seem to fail due to one of the usual sendable changes. @0xTim feel free to let me know if any improvements can be done. |
CODE/Sources/PennySHARED/Repositories/DynamoUserRepository.swift
Outdated
Show resolved
Hide resolved
| import Logging | ||
| import Foundation | ||
|
|
||
| public class FakeAWSHTTPClient: AWSHTTPClient { |
There was a problem hiding this comment.
o hmm, I don't follow AWS SDKs closely, but I can see that they're removing it: soto-project/soto-core@addcfe6
I think we might need to make a "AWSClientProtocol" of ourselves now?
There was a problem hiding this comment.
oo the thing is we're still using soto 5 instead of the newest soto 6 ... that's why they are removing the protocol (since it no longer has any use in soto 6)
There was a problem hiding this comment.
We can update Soto in a separate PR. Where are you abstracting the AWSClient? (I'm on mobile). We probably want to move the abstraction a layer up rather to eg a EC2Interface rather than the client directly. That will make it easier to migrate Soto versions too
There was a problem hiding this comment.
So that'll be a "AWSClientProtocol" of ourselves again? This way we can hide the AWS interface under a protocol of ourselves and only add functions we need (which makes updating easier too)
There was a problem hiding this comment.
Sorry it appears Github doesn't update the conversation unless you refresh the page, so i missed your last comment.
There was a problem hiding this comment.
Replying to your last comment:
Yeah i was thinking of that too a while ago (although i forgot about it later)... i'll see what i can do
There was a problem hiding this comment.
Just pushed the changes.
I didn't find a better solution than the underlyingHandler solution.
We can simplify these after we update our soto stuff, including the runtime.
There was a problem hiding this comment.
Ok i just noticed something ... let me check ...
There was a problem hiding this comment.
Pushed some more changes to simplify those stuff
|
Its interesting that the tests randomly throw
once in a while, even in Swift 5.6 (other than nightly) ... this doesn't happen at all on macOS |
|
Why not just drop 5.6 and move to 5.7? |
|
we definitely should 😅 |
|
Yep sounds good |
|
All ready! |
CODE/Tests/Fake/FakeManager.swift
Outdated
| } | ||
| } | ||
|
|
||
| private struct FakeDiscordClient: DiscordClient { |
There was a problem hiding this comment.
It would be nice to split these out into their own types
There was a problem hiding this comment.
I assume you mean the send and sendAndAwaitResponse functions?
There was a problem hiding this comment.
Sorry I meant files - FakeDiscordClient and FakeResponseStorage should probably live in their own files
Problem
Penny needs better tests so we can put more trust in it to be working after each update.
Some Explanation
I don't intend to add a lot of tests in this PR.
What I want to do is to prepare the abstractions and clear up the way.
Then anybody will be able to write some tests without a need to worry about more complex stuff.