Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 167fa6db4c1b
Choose a base ref
...
head repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 5b1b9fbacb52
Choose a head ref
  • 7 commits
  • 9 files changed
  • 1 contributor

Commits on Oct 12, 2021

  1. Remove a layer of indirection.

    pleroy committed Oct 12, 2021
    Copy the full SHA
    cbb4755 View commit details

Commits on Oct 13, 2021

  1. Copy the full SHA
    cfd03ba View commit details
  2. Fix a test.

    pleroy committed Oct 13, 2021
    Copy the full SHA
    a2bf91d View commit details
  3. All tests passing.

    pleroy committed Oct 13, 2021
    Copy the full SHA
    ad5f0c2 View commit details
  4. Dead code.

    pleroy committed Oct 13, 2021
    Copy the full SHA
    5ffe9c7 View commit details
  5. Merge.

    pleroy committed Oct 13, 2021
    Copy the full SHA
    9903b39 View commit details
  6. Merge pull request #3152 from pleroy/Pointer

    Cleanup and simplification of new-style discrete trajectory classes
    pleroy authored Oct 13, 2021
    Copy the full SHA
    5b1b9fb View commit details
152 changes: 52 additions & 100 deletions physics/discrete_trajectory_iterator_test.cpp
Original file line number Diff line number Diff line change
@@ -12,135 +12,87 @@
#include "physics/discrete_trajectory_segment.hpp"
#include "physics/discrete_trajectory_segment_iterator.hpp"
#include "physics/discrete_trajectory_types.hpp"
#include "physics/mock_discrete_trajectory_segment.hpp"
#include "quantities/quantities.hpp"
#include "quantities/si.hpp"

namespace principia {
namespace physics {

using base::check_not_null;
using base::make_not_null_unique;
using base::not_null;
using geometry::Frame;
using geometry::Instant;
using physics::DegreesOfFreedom;
using quantities::Time;
using quantities::si::Second;
using ::testing::Return;

namespace {

// A trajectory that holds a real timeline, where we mock multiple methods to
// return data from that timeline.
template<typename Frame>
class FakeDiscreteTrajectorySegment
: public MockDiscreteTrajectorySegment<Frame> {
public:
FakeDiscreteTrajectorySegment() = default;

internal_discrete_trajectory_types::Timeline<Frame> timeline;
};

} // namespace

class DiscreteTrajectoryIteratorTest : public ::testing::Test {
protected:
using World = Frame<enum class WorldTag>;

using Segments = internal_discrete_trajectory_types::Segments<World>;
using Timeline = internal_discrete_trajectory_types::Timeline<World>;

DiscreteTrajectoryIteratorTest() {
// Set up a fake trajectory with 3 segments. After construction, the mocks
// are owned by |segments_|.
auto owned_mock1 = std::make_unique<FakeDiscreteTrajectorySegment<World>>();
auto owned_mock2 = std::make_unique<FakeDiscreteTrajectorySegment<World>>();
auto owned_mock3 = std::make_unique<FakeDiscreteTrajectorySegment<World>>();
auto const& mock1 = *owned_mock1;
auto const& mock2 = *owned_mock2;
auto const& mock3 = *owned_mock3;

segments_.push_back(std::move(owned_mock1));
auto const it1 = --segments_.end();
segments_.push_back(std::move(owned_mock2));
auto const it2 = --segments_.end();
segments_.push_back(std::move(owned_mock3));
auto const it3 = --segments_.end();

FillSegment(it1,
Timeline{MakeTimelineValueType(2 * Second),
MakeTimelineValueType(3 * Second),
MakeTimelineValueType(5 * Second),
MakeTimelineValueType(7 * Second),
MakeTimelineValueType(11 * Second)});
FillSegment(it2, Timeline{MakeTimelineValueType(13 * Second)});
FillSegment(it3,
Timeline{MakeTimelineValueType(13 * Second), // Duplicated.
MakeTimelineValueType(17 * Second),
MakeTimelineValueType(19 * Second),
MakeTimelineValueType(23 * Second)});

// This must happen *after* the segments have been set up.
EXPECT_CALL(mock1, timeline_begin())
.WillRepeatedly(Return(timeline_begin(it1)));
EXPECT_CALL(mock1, timeline_end())
.WillRepeatedly(Return(timeline_end(it1)));
EXPECT_CALL(mock2, timeline_begin())
.WillRepeatedly(Return(timeline_begin(it2)));
EXPECT_CALL(mock2, timeline_end())
.WillRepeatedly(Return(timeline_end(it2)));
EXPECT_CALL(mock3, timeline_begin())
.WillRepeatedly(Return(timeline_begin(it3)));
EXPECT_CALL(mock3, timeline_end())
.WillRepeatedly(Return(timeline_end(it3)));
}

FakeDiscreteTrajectorySegment<World>* DownCast(
std::unique_ptr<DiscreteTrajectorySegment<World>> const& segment) {
return dynamic_cast<FakeDiscreteTrajectorySegment<World>*>(segment.get());
}

void FillSegment(Segments::iterator const it, Timeline const& timeline) {
auto* const segment = DownCast(*it);
segment->timeline = timeline;
DiscreteTrajectoryIteratorTest()
: segments_(MakeSegments(3)) {
auto it = segments_->begin();
{
auto& segment1 = *it;
segment1.Append(t0_ + 2 * Second, unmoving_origin_);
segment1.Append(t0_ + 3 * Second, unmoving_origin_);
segment1.Append(t0_ + 5 * Second, unmoving_origin_);
segment1.Append(t0_ + 7 * Second, unmoving_origin_);
segment1.Append(t0_ + 11 * Second, unmoving_origin_);
}

++it;
{
auto& segment2 = *it;
segment2.Append(t0_ + 13 * Second, unmoving_origin_);
}

++it;
{
auto& segment3 = *it;
segment3.Append(t0_ + 13 * Second, unmoving_origin_);
segment3.Append(t0_ + 17 * Second, unmoving_origin_);
segment3.Append(t0_ + 19 * Second, unmoving_origin_);
segment3.Append(t0_ + 23 * Second, unmoving_origin_);
}
}

DiscreteTrajectoryIterator<World> MakeBegin(
Segments::const_iterator const it) {
return DiscreteTrajectoryIterator<World>(
DiscreteTrajectorySegmentIterator<World>(
check_not_null(&segments_), it),
timeline_begin(it));
DiscreteTrajectorySegmentIterator<World>(segments_.get(), it),
it->timeline_begin());
}

DiscreteTrajectoryIterator<World> MakeEnd(Segments::const_iterator it) {
return DiscreteTrajectoryIterator<World>(
DiscreteTrajectorySegmentIterator<World>(check_not_null(&segments_),
++it),
DiscreteTrajectorySegmentIterator<World>(segments_.get(), ++it),
std::nullopt);
}

Timeline::value_type MakeTimelineValueType(Time const& t) {
static const DegreesOfFreedom<World> unmoving_origin(World::origin,
World::unmoving);
return {t0_ + t, unmoving_origin};
}

internal_discrete_trajectory_types::Timeline<World>::const_iterator
timeline_begin(Segments::const_iterator const it) {
return DownCast(*it)->timeline.begin();
}

internal_discrete_trajectory_types::Timeline<World>::const_iterator
timeline_end(Segments::const_iterator const it) {
return DownCast(*it)->timeline.end();
// Constructs a list of |n| segments which are properly initialized.
// TODO(phl): Move to a central place.
static not_null<std::unique_ptr<Segments>> MakeSegments(const int n) {
auto segments = make_not_null_unique<Segments>(n);
for (auto it = segments->begin(); it != segments->end(); ++it) {
*it = DiscreteTrajectorySegment<World>(
DiscreteTrajectorySegmentIterator<World>(segments.get(), it));
}
return segments;
}

Segments segments_;
not_null<std::unique_ptr<Segments>> segments_;
Instant const t0_;
DegreesOfFreedom<World> const unmoving_origin_{World::origin,
World::unmoving};
};

TEST_F(DiscreteTrajectoryIteratorTest, ForwardOneSegment) {
auto segment = segments_.begin();
auto segment = segments_->begin();
auto iterator = MakeBegin(segment);
EXPECT_EQ(t0_ + 2 * Second, iterator->first);
auto const current = ++iterator;
@@ -152,7 +104,7 @@ TEST_F(DiscreteTrajectoryIteratorTest, ForwardOneSegment) {
}

TEST_F(DiscreteTrajectoryIteratorTest, BackwardOneSegment) {
auto segment = --segments_.end();
auto segment = --segments_->end();
auto iterator = MakeEnd(segment);
--iterator;
EXPECT_EQ(t0_ + 23 * Second, (*iterator).first);
@@ -165,7 +117,7 @@ TEST_F(DiscreteTrajectoryIteratorTest, BackwardOneSegment) {
}

TEST_F(DiscreteTrajectoryIteratorTest, ForwardAcrossSegments) {
auto segment = segments_.begin();
auto segment = segments_->begin();
auto iterator = MakeBegin(segment);
for (int i = 0; i < 4; ++i) {
++iterator;
@@ -178,7 +130,7 @@ TEST_F(DiscreteTrajectoryIteratorTest, ForwardAcrossSegments) {
}

TEST_F(DiscreteTrajectoryIteratorTest, BackwardAcrossSegments) {
auto segment = --segments_.end();
auto segment = --segments_->end();
auto iterator = MakeEnd(segment);
for (int i = 0; i < 3; ++i) {
--iterator;
@@ -193,15 +145,15 @@ TEST_F(DiscreteTrajectoryIteratorTest, BackwardAcrossSegments) {
TEST_F(DiscreteTrajectoryIteratorTest, Equality) {
// Construct two iterators that denote the time 13 * Second but in different
// segments.
auto it1 = MakeEnd(--segments_.end());
auto it1 = MakeEnd(--segments_->end());
for (int i = 0; i < 3; ++i) {
--it1;
}
EXPECT_EQ(t0_ + 17 * Second, (*it1).first);
--it1;
EXPECT_EQ(t0_ + 13 * Second, (*it1).first);

auto it2 = MakeBegin(segments_.begin());
auto it2 = MakeBegin(segments_->begin());
for (int i = 0; i < 4; ++i) {
++it2;
}
@@ -210,9 +162,9 @@ TEST_F(DiscreteTrajectoryIteratorTest, Equality) {
EXPECT_EQ(t0_ + 13 * Second, it2->first);

EXPECT_EQ(it1, it2);
EXPECT_NE(it1, MakeBegin(segments_.begin()));
EXPECT_NE(it2, MakeEnd(--segments_.end()));
EXPECT_NE(MakeBegin(segments_.begin()), MakeEnd(--segments_.end()));
EXPECT_NE(it1, MakeBegin(segments_->begin()));
EXPECT_NE(it2, MakeEnd(--segments_->end()));
EXPECT_NE(MakeBegin(segments_->begin()), MakeEnd(--segments_->end()));
}

} // namespace physics
2 changes: 2 additions & 0 deletions physics/discrete_trajectory_segment.hpp
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@ FORWARD_DECLARE_FROM(discrete_trajectory_factories,
namespace physics {

class DiscreteTrajectoryIteratorTest;
class DiscreteTrajectorySegmentIteratorTest;
class DiscreteTrajectorySegmentTest;

namespace internal_discrete_trajectory_segment {
@@ -137,6 +138,7 @@ class DiscreteTrajectorySegment : public Trajectory<Frame> {

// For testing.
friend class physics::DiscreteTrajectoryIteratorTest;
friend class physics::DiscreteTrajectorySegmentIteratorTest;
friend class physics::DiscreteTrajectorySegmentTest;
template<typename F>
friend class testing_utilities::DiscreteTrajectoryFactoriesFriend;
4 changes: 2 additions & 2 deletions physics/discrete_trajectory_segment_iterator_body.hpp
Original file line number Diff line number Diff line change
@@ -33,13 +33,13 @@ DiscreteTrajectorySegmentIterator<Frame>::operator--(int) { // NOLINT
template<typename Frame>
internal_discrete_trajectory_segment::DiscreteTrajectorySegment<Frame> const&
DiscreteTrajectorySegmentIterator<Frame>::operator*() const {
return **iterator_;
return *iterator_;
}

template<typename Frame>
internal_discrete_trajectory_segment::DiscreteTrajectorySegment<Frame> const*
DiscreteTrajectorySegmentIterator<Frame>::operator->() const {
return iterator_->get();
return &*iterator_;
}

template<typename Frame>
72 changes: 51 additions & 21 deletions physics/discrete_trajectory_segment_iterator_test.cpp
Original file line number Diff line number Diff line change
@@ -5,53 +5,83 @@

#include "base/not_null.hpp"
#include "geometry/frame.hpp"
#include "geometry/named_quantities.hpp"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "physics/discrete_trajectory_segment.hpp"
#include "physics/discrete_trajectory_types.hpp"
#include "physics/mock_discrete_trajectory_segment.hpp"
#include "quantities/si.hpp"

namespace principia {
namespace physics {

using base::check_not_null;
using base::make_not_null_unique;
using base::not_null;
using geometry::Frame;
using geometry::Instant;
using quantities::si::Second;
using ::testing::Return;

// We use a mock segment in this test to avoid having to go through a
// complicated setup just to test the iterator.
class DiscreteTrajectorySegmentIteratorTest : public ::testing::Test {
protected:
using World = Frame<enum class WorldTag>;

using Segments = internal_discrete_trajectory_types::Segments<World>;

DiscreteTrajectorySegmentIteratorTest()
: segments_(MakeSegments(3)) {
auto it = segments_->begin();
{
auto& segment1 = *it;
segment1.Append(t0_ + 2 * Second, unmoving_origin_);
segment1.Append(t0_ + 3 * Second, unmoving_origin_);
segment1.Append(t0_ + 5 * Second, unmoving_origin_);
segment1.Append(t0_ + 7 * Second, unmoving_origin_);
segment1.Append(t0_ + 11 * Second, unmoving_origin_);
}

++it;
{
auto& segment2 = *it;
segment2.Append(t0_ + 13 * Second, unmoving_origin_);
}

++it;
{
auto& segment3 = *it;
segment3.Append(t0_ + 13 * Second, unmoving_origin_);
segment3.Append(t0_ + 17 * Second, unmoving_origin_);
segment3.Append(t0_ + 19 * Second, unmoving_origin_);
}
}

DiscreteTrajectorySegmentIterator<World> MakeIterator(
not_null<Segments const*> const segments,
Segments::const_iterator const iterator) {
return DiscreteTrajectorySegmentIterator<World>(segments, iterator);
}
};

TEST_F(DiscreteTrajectorySegmentIteratorTest, Basic) {
auto owned_mock1 = std::make_unique<MockDiscreteTrajectorySegment<World>>();
auto owned_mock2 = std::make_unique<MockDiscreteTrajectorySegment<World>>();
auto owned_mock3 = std::make_unique<MockDiscreteTrajectorySegment<World>>();
auto const& mock1 = *owned_mock1;
auto const& mock2 = *owned_mock2;
auto const& mock3 = *owned_mock3;

Segments segments;
segments.push_back(std::move(owned_mock1));
segments.push_back(std::move(owned_mock2));
segments.push_back(std::move(owned_mock3));
// Constructs a list of |n| segments which are properly initialized.
// TODO(phl): Move to a central place.
static not_null<std::unique_ptr<Segments>> MakeSegments(const int n) {
auto segments = make_not_null_unique<Segments>(n);
for (auto it = segments->begin(); it != segments->end(); ++it) {
*it = DiscreteTrajectorySegment<World>(
DiscreteTrajectorySegmentIterator<World>(segments.get(), it));
}
return segments;
}

EXPECT_CALL(mock1, size()).WillRepeatedly(Return(5));
EXPECT_CALL(mock2, size()).WillRepeatedly(Return(1));
EXPECT_CALL(mock3, size()).WillRepeatedly(Return(3));
not_null<std::unique_ptr<Segments>> segments_;
Instant const t0_;
DegreesOfFreedom<World> const unmoving_origin_{World::origin,
World::unmoving};
};

TEST_F(DiscreteTrajectorySegmentIteratorTest, Basic) {
{
auto iterator = MakeIterator(check_not_null(&segments), segments.begin());
auto iterator = MakeIterator(segments_.get(), segments_->begin());
EXPECT_EQ(5, iterator->size());
auto const current = ++iterator;
EXPECT_EQ(1, iterator->size());
@@ -61,7 +91,7 @@ TEST_F(DiscreteTrajectorySegmentIteratorTest, Basic) {
EXPECT_EQ(1, previous->size());
}
{
auto iterator = MakeIterator(check_not_null(&segments), segments.end());
auto iterator = MakeIterator(segments_.get(), segments_->end());
--iterator;
EXPECT_EQ(3, (*iterator).size());
auto const current = --iterator;
Loading