FabioBatSilva / ArduinoFake

Arduino mocking made easy
https://platformio.org/lib/show/1689/ArduinoFake
MIT License
105 stars 48 forks source link

fix: update exception handling to support gnu++17 #41

Closed RobertByrnes closed 1 year ago

RobertByrnes commented 1 year ago

Add __cplusplus version check an and use std::uncaught_exceptions() if __cplusplus version is greater than or equal to 201703L . This is to support compilation with gnu++17 compiler.

RobertByrnes commented 1 year ago

Hi, I was building this to do some mocking of TinyGSM in native environment on windows. I needed so C++ language feature from gnu++14 and also noticed compiler warnings for the deprecated std::uncaught_exception() function.

FabioBatSilva commented 1 year ago

Thanks @RobertByrnes

That file is just a dump of https://github.com/eranpeer/FakeIt/tree/master/single_header/standalone

I mean to pull it as a dependency but never got to it.. https://github.com/FabioBatSilva/ArduinoFake/tree/master/src/fakeit#readme

I see some reference to __cplusplus in the latest version of fakeit it might be fixed upstream.. I think it might be better to update the entirely file to the latest stable version instead of maintaining it here..

RobertByrnes commented 1 year ago

Hi, I did check out the latest version of fakeit.hpp and yes they did fix it upstream! I didn't realise until after I submitted the PR and then thought yes better to pull upstream than get divergent here.

Ultimate fix as per your readme to use fakeit as dep for ArduinoFake...

Happy to implement and submit new PR if you would like :)

RobertByrnes commented 1 year ago

...for now have updated entire file to upstream FakeIt/fakeit.hpp

FabioBatSilva commented 1 year ago

Awesome..

PR making it a dependency would be greatly appreciated.. But ok to merge this one first..

Looks like there are some breaking changes. https://github.com/FabioBatSilva/ArduinoFake/actions/runs/4246816134/jobs/7385515857#step:3:83 https://github.com/FabioBatSilva/ArduinoFake/blob/master/test/test_print.h#L40

Maybe something specific to how that type or method signature is handled by fakeit now.. :thinking:

RobertByrnes commented 1 year ago

Hello again, i'll look at adding it as a dependency tomorrow. I saw that there were failed test since updating the entire file to fakeit's latest version so I agree about breaking changes....

More to follow .... 🙂

RobertByrnes commented 1 year ago

Hi Fabio,

Client is OK now also. There is something about the way fakeit is handling the strings. Something to do with scope / out of scope in both Print and Client classes.

I am still a little concerned by the Print class fix, having changed your test data type from char[] to const char *. It works but this may be a breaking change for ArduinoFake? If so the README.md and semver will need a bump? Or a better fix 😀

FabioBatSilva commented 1 year ago

@RobertByrnes Thanks for dinging into it. :+1: Yes, would be a breaking change, with new major version if we need to do this..

Let me investigate it a bit about it before we merge it..

RobertByrnes commented 1 year ago

Hi, just playing with pulling it in as a dependency will re sumbit PR on success