Page MenuHomeWildfire Games
Differential D4718 Diff 20601 source/simulation2/tests/test_ComponentDataHolder.h

Changeset View

Standalone View

View Options

source/simulation2/tests/test_ComponentDataHolder.h

  • This file was added.
/* Copyright (C) 2021 Wildfire Games.
Lint: Inaccurate Copyright Year
Inline Actions

Inaccurate Copyright Year

Lint: Inaccurate Copyright Year: Inaccurate Copyright Year
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2 of the License, or
* (at your option) any later version.
*
* 0 A.D. is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with 0 A.D. If not, see <http://www.gnu.org/licenses/>.
*/
#include "lib/self_test.h"
#include "simulation2/system/Components.h"
#include "simulation2/system/IComponent.h"
StanUnsubmitted
Done
Inline Actions

Anything that can be forward declared?
Bit weird not to have CComponentDataHolder.h isn't it?

Stan: Anything that can be forward declared? Bit weird not to have CComponentDataHolder.h isn't it?
MercuryAuthorUnsubmitted
Done
Inline Actions

Added include for ComponentDataHolder.h

What would we want to forward declare?

Mercury: Added include for ComponentDataHolder.h What would we want to forward declare?
#include "ps/CLogger.h"
StanUnsubmitted
Done
Inline Actions

Does that print something? If so, it should use TestLogger, and probably some checks.

Stan: Does that print something? If so, it should use TestLogger, and probably some checks.
MercuryAuthorUnsubmitted
Done
Inline Actions

I'm not sure I understand the question, but the include does not print anything.

Mercury: I'm not sure I understand the question, but the include does not print anything.
StanUnsubmitted
Done
Inline Actions

I mean does the code in the test use CLogger somewhere, if so, can we add assertion to check whatever its printing. E.g error message etc.

Stan: I mean does the code in the test use CLogger somewhere, if so, can we add assertion to check…
MercuryAuthorUnsubmitted
Done
Inline Actions

Ah, no it doesn't.

Mercury: Ah, no it doesn't.
StanUnsubmitted
Done
Inline Actions

It's a bit strange that you need that header then.

Stan: It's a bit strange that you need that header then.
MercuryAuthorUnsubmitted
Done
Inline Actions

If the logger isn't initialized the test system fails an ensure after the tests complete. Does seem odd.

Mercury: If the logger isn't initialized the test system fails an ensure after the tests complete. Does…
#include <cstddef>
class TestComponentDataHolder : public CxxTest::TestSuite
{
jprahmanUnsubmitted
Done
Inline Actions

I'd consider tests for:

  1. Multiple allocations then multiple deallocations
  2. Allocation -> Deallocation -> Allocation -> Deallocation -> ...
  3. Large numbers of allocations which require setting up new pools
jprahman: I'd consider tests for: 1. Multiple allocations then multiple deallocations 2. Allocation ->…
public:
void test_SetUp()
{
CComponentDataHolder cdh = CComponentDataHolder(32,8);
TS_ASSERT_EQUALS(cdh.Start(), cdh.Back());
}
void test_Allocate()
{
CComponentDataHolder cdh = CComponentDataHolder(32,8);
std::byte* ptr = cdh.Allocate();
TS_ASSERT_EQUALS(ptr, cdh.Start());
TS_ASSERT_EQUALS(cdh.Start() + cdh.Offset(), cdh.Back());
}
void test_Deallocate()
{
CComponentDataHolder cdh = CComponentDataHolder(32,8);
std::byte* ptr = cdh.Allocate();
cdh.Deallocate(reinterpret_cast<IComponent*>(ptr));
std::byte* ptr2 = cdh.Allocate();
TS_ASSERT_EQUALS(ptr, ptr2);// verify slot is reused
}
StanUnsubmitted
Done
Inline Actions
std::byte* ptr2 = cdh.Allocate();
- TS_ASSERT_EQUALS(ptr, ptr2);// verify slot is reused
+ // Verify slot is reused.
+ TS_ASSERT_EQUALS(ptr, ptr2);
}
};
Stan:
StanUnsubmitted
Done
Inline Actions

Caps and final dot please, and no whitespace :D

Stan: Caps and final dot please, and no whitespace :D
};
StanUnsubmitted
Done
Inline Actions

Think you need an include for that. #include <cstdint>

++i in the whole file.
space after for.

Stan: Think you need an include for that. `#include <cstdint>` ++i in the whole file. space after…
MercuryAuthorUnsubmitted
Done
Inline Actions

does it actually make a difference to use ++i vs. i++? Seems like something the compiler should be able to handle.

Mercury: does it actually make a difference to use ++i vs. i++? Seems like something the compiler should…
StanUnsubmitted
Done
Inline Actions

In most cases no. For non trivial types it can prevent a copy but we mostly do it as a convention rather than an actual perf boost.
See https://stackoverflow.com/questions/2020184/preincrement-faster-than-postincrement-in-c-true-if-yes-why-is-it#2020205

Stan: In most cases no. For non trivial types it can prevent a copy but we mostly do it as a…
Wildfire Games · Phabricator