Conversation
|
|
AdityaKasar
force-pushed
the
test/mock-tests
branch
from
1d97061 to
87683e9
Compare
| Transport/Transport.cpp | ||
| Accessor/Accessor.cpp | ||
| Async/Async.cpp | ||
| Properties/Properties.cpp |
There was a problem hiding this comment.
not needed, all methods in Propierties class are static and should remain static. Tests should verify code as it is
|
|
||
| void TransportUpdated(Transport<WPEFramework::Core::JSON::IElement>* transport); | ||
|
|
||
| void UpdateGateway(std::unique_ptr<GatewayImpl> mockGateway) |
There was a problem hiding this comment.
as it is only for tests, should be at least under #ifdef ... #endif block, not exposed for any other application using the API.
There was a problem hiding this comment.
Several things:
- Application class MUST NOT inherit from a class created for testing purposes. You should make tests for sources as they are without adopting them to your needs
- Keep existing formatting as it is, it makes comparison easier, I am referring to
- Moving brackets to new lines
- Changing argument convention from
type& argtotype &arg, I am not arguing here which convention is better, just it is out of the scope of this task
There was a problem hiding this comment.
I agree for the formatting changes, I think my editors autoformator might have made that. I will try to revert it
There was a problem hiding this comment.
That class cannot be a part of the application, it has been create for fulfill test scenarios, so its place is in test/ in SDK.
There was a problem hiding this comment.
That class cannot be a part of the application, it has been create for fulfill test scenarios, so its place is in test/ in SDK.
There was a problem hiding this comment.
That class cannot be a part of the application, it has been create for fulfill test scenarios, so its place is in test/ in SDK.
There was a problem hiding this comment.
That class cannot be a part of the application, it has been create for fulfill test scenarios, so its place is in test/ in SDK.
There was a problem hiding this comment.
Please, do not modify existing sources to fulfil test requirements.
There was a problem hiding this comment.
Please, do not modify existing sources to make tests working.
Also, the same as for Gateway, application class MUST NOT inherit from mock classes created just for testing purposes.
There was a problem hiding this comment.
If Properties class won't be changes to make tests, that change would be not needed.
tomasz-blasz
left a comment
tomasz-blasz
left a comment
There was a problem hiding this comment.
Overall, mock classes for testing purposes cannot exist at application level AND application classes cannot inherit from them.
AdityaKasar
force-pushed
the
test/mock-tests
branch
from
b2f171e to
cbe7f11
Compare
AdityaKasar
force-pushed
the
test/mock-tests
branch
from
2652d4b to
067a131
Compare
3 participants
No description provided.