> The reasoning for doing it doesn't sound great. Doing DeepEquals on
such
> a structure is most definitely wrong. Private data is private, and by
> definition we should be able to cache information freely or change it
> without breaking tests that work on the public APIs.
It may be wrong, but in that case we have to do lots of work to fix it
because all the documents will have this field.
white:mstate$ cd state
/home/aram/src/launchpad.net/juju-core/state
white:state$ g DeepEquals | wc -l
89
white:state$ cd mstate
/home/aram/src/launchpad.net/juju-core/mstate
white:mstate$ g DeepEquals | wc -l
58
To be honest, using DeepEquals in tests always felt wrong to be because
it was checking the implementation details instead of relying on the
interface, but I assumed it was ok since this is the way state tests
were written in the first place.
I will put omitempty, but this doesn't fix the tests. Entities returned
by Insert will have TxtRevno always zero but entities returned by other
querying functions or by Refresh will have a non-zero TxtRevno.
If you are against this change, perhaps an alternative is to call
Refresh() in tests where this might matter.
> The reasoning for doing it doesn't sound great. Doing DeepEquals on
such
> a structure is most definitely wrong. Private data is private, and by
> definition we should be able to cache information freely or change it
> without breaking tests that work on the public APIs.
It may be wrong, but in that case we have to do lots of work to fix it
because all the documents will have this field.
white:mstate$ cd state aram/src/ launchpad. net/juju- core/state aram/src/ launchpad. net/juju- core/mstate
/home/
white:state$ g DeepEquals | wc -l
89
white:state$ cd mstate
/home/
white:mstate$ g DeepEquals | wc -l
58
To be honest, using DeepEquals in tests always felt wrong to be because
it was checking the implementation details instead of relying on the
interface, but I assumed it was ok since this is the way state tests
were written in the first place.
I will put omitempty, but this doesn't fix the tests. Entities returned
by Insert will have TxtRevno always zero but entities returned by other
querying functions or by Refresh will have a non-zero TxtRevno.
If you are against this change, perhaps an alternative is to call
Refresh() in tests where this might matter.
https:/ /codereview. appspot. com/6489104/