Make sure the tests in attic don't depend on the new testlib
ClosedPublic
Actions

Authored by ervin on Aug 28 2015, 1:10 PM.

Details

Reviewers

Commits

Repository

R4 Zanshin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

ervin retitled this revision from to Make sure the tests in attic don't depend on the new testlib.

ervin updated this object.

ervin edited the test plan for this revision. (Show Details)

bensi requested changes to this revision.Sep 2 2015, 8:31 AM

bensi edited edge metadata.

bensi added inline comments.

attic/tests/testlib/modeltest.cpp
158	we should remove this no ?
202	ditto
227	ditto
257	ditto
337	ditto
369	ditto ?
378	ditto
384	ditto
389	ditto
479	ditto
495	& parent -> &parent
499	we should remove this no ?
556	& parent -> &parent
attic/tests/testlib/modeltest.h
72	& parent => &parent to have the same thing everywhere
74	ditto

This revision now requires changes to proceed.Sep 2 2015, 8:31 AM

For the record, I'm not going to change modeltest.*, for the following reasons:

it is a copied class (we have another copy in tests/testlib);
this copy is temporary, it will go away with attic;
it is third party code we didn't write (as can be seen from the copyright headers).

Could you reconsider your vote on that patch in light of the above?

Thanks.

bensi edited edge metadata.

This revision is now accepted and ready to land.Sep 3 2015, 7:34 AM

This revision was automatically updated to reflect the committed changes.

		Path
M		attic/tests/CMakeLists.txt (10 lines)
M		attic/tests/testlib/CMakeLists.txt (1 line)
A	M	attic/tests/testlib/modeltest.h (94 lines)
A	M	attic/tests/testlib/modeltest.cpp (564 lines)