markembling / MarkEmbling.PostcodesIO

.NET library for interacting with the excellent Postcodes.io service.
MIT License
23 stars 16 forks source link

Poor testability without hitting live API #16

Open markembling opened 5 years ago

markembling commented 5 years ago

Right now, testing is pretty much reliant on hitting the live API in integration tests. This needs to change so all the independent bits of behaviour (e.g. deserialisation of results) does not depend on hitting the live API and hard-coding live data which may change over time.

May have an impact on the public API, so ideally would happen before v1.0.0.

SeanFarrow commented 3 years ago

Has any of this been done.

I can see other benefits: Allow the use of HttpClient as well as System.Text.JSON. Allow the use of Polly for retries.

I've got a codebase that is a AWS lambda function that by default uses System.Text.JSON for deserialization and HttpClient to deal with HTTP calls. I have a need to use this repo for postcode validation, but hte previous limitations a\e meaning I['m brining in more packages (newtonsoft.json and RestSharp) than I really need too. I'm happy to help where I can making this more testable.

markembling commented 3 years ago

I made a start ages ago on a bunch of stuff which, IIRC, would have been the first steps to this issue but honestly it's been so long that I'd need to refresh myself as to what I was actually doing there. It's the stuff in the vnext-wip branch. But beyond that, no, no progress - the last couple of years have left me with little time unfortunately and this all got a bit left behind.

SeanFarrow commented 3 years ago

I’m happy to fork and look at that branch if it would help?

From: Mark Embling @.> Sent: 15 August 2021 11:58 To: markembling/MarkEmbling.PostcodesIO @.> Cc: Sean Farrow @.>; Comment @.> Subject: Re: [markembling/MarkEmbling.PostcodesIO] Poor testability without hitting live API (#16)

I made a start ages ago on a bunch of stuff which, IIRC, would have been the first steps to this issue but honestly it's been so long that I'd need to refresh myself as to what I was actually doing there. It's the stuff in the vnext-wip branch. But beyond that, no, no progress - the last couple of years have left me with little time unfortunately and this all got a bit left behind.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/markembling/MarkEmbling.PostcodesIO/issues/16#issuecomment-899032345, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7WYIH2BUI3V47GR27LT46MRXANCNFSM4HGW4INQ.

markembling commented 3 years ago

Please do 🙂

SeanFarrow commented 3 years ago

Ok, I will.

Do we need to keep the synchronous API? I can raise a separate issue for this if needed.

Thanks, Sean.

From: Mark Embling @.> Sent: 15 August 2021 12:10 To: markembling/MarkEmbling.PostcodesIO @.> Cc: Sean Farrow @.>; Comment @.> Subject: Re: [markembling/MarkEmbling.PostcodesIO] Poor testability without hitting live API (#16)

Please do 🙂

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/markembling/MarkEmbling.PostcodesIO/issues/16#issuecomment-899033942, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7VSFMJYBEBL6LBNKALT46N7BANCNFSM4HGW4INQ.

markembling commented 3 years ago

I think it would be good to keep that if possible but I'm not wedded to it. I could be wrong, but I think the only place it'd really matter is older .NET Framework apps which use it. Since this isn't at 1.0 yet, I feel that do have flexibility in terms of breaking changes if it makes sense going forward.

SeanFarrow commented 3 years ago

I would remove it for the following reasons: Firstly, both the target frameworks support async as a first class citizen. Secondly, given the library makes network calls, async makes sense as it wouldn’t (for example) block the UI thread.

Just my $.02 cents worth. From: Mark Embling @.> Sent: 15 August 2021 13:39 To: markembling/MarkEmbling.PostcodesIO @.> Cc: Sean Farrow @.>; Comment < @.> Subject: Re: [markembling/MarkEmbling.PostcodesIO] Poor testability without hitting live API (#16)

I think it would be good to keep that if possible but I'm not wedded to it. I could be wrong, but I think the only place it'd really matter is older .NET Framework apps which use it. Since this isn't at 1.0 yet, I feel that do have flexibility in terms of breaking changes if it makes sense going forward.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/markembling/MarkEmbling.PostcodesIO/issues/16#issuecomment-899044368, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7RYJYGKMRLWPVJKCXTT46YM5ANCNFSM4HGW4INQ.