Skip to content

Commit

Permalink
#5643: Start setting up the unit tests for the three-way merge scenario
Browse files Browse the repository at this point in the history
  • Loading branch information
codereader committed Jun 15, 2021
1 parent 33d28ff commit 39f57ac
Show file tree
Hide file tree
Showing 8 changed files with 5,837 additions and 6 deletions.
5 changes: 5 additions & 0 deletions libs/scene/merge/MergeOperationBase.cpp
Expand Up @@ -36,6 +36,11 @@ void MergeOperationBase::foreachAction(const std::function<void(const IMergeActi
}
}

void MergeOperationBase::clearActions()
{
_actions.clear();
}

void MergeOperationBase::addActionForKeyValueDiff(const ComparisonResult::KeyValueDifference& difference,
const scene::INodePtr& targetEntity)
{
Expand Down
1 change: 1 addition & 0 deletions libs/scene/merge/MergeOperationBase.h
Expand Up @@ -25,6 +25,7 @@ class MergeOperationBase :

protected:
virtual void addAction(const MergeAction::Ptr& action);
void clearActions();

void addActionForKeyValueDiff(const ComparisonResult::KeyValueDifference& difference,
const scene::INodePtr& targetEntity);
Expand Down
6 changes: 6 additions & 0 deletions libs/scene/merge/ThreeWayMergeOperation.cpp
Expand Up @@ -16,6 +16,12 @@ ThreeWayMergeOperation::ThreeWayMergeOperation(const scene::IMapRootNodePtr& bas
_targetRoot(targetRoot)
{}

ThreeWayMergeOperation::~ThreeWayMergeOperation()
{
// Clear the actions held by the base class before the root nodes are cleared
clearActions();
}

std::list<ComparisonResult::KeyValueDifference>::const_iterator ThreeWayMergeOperation::FindTargetDiffByKey(
const std::list<ComparisonResult::KeyValueDifference>& targetKeyValueDiffs, const std::string& key)
{
Expand Down
22 changes: 17 additions & 5 deletions libs/scene/merge/ThreeWayMergeOperation.h
Expand Up @@ -22,9 +22,9 @@ class ThreeWayMergeOperation :
public MergeOperationBase
{
private:
scene::IMapRootNodePtr _baseRoot; // a common ancestor of the two maps
scene::IMapRootNodePtr _sourceRoot; // the map to be merged
scene::IMapRootNodePtr _targetRoot; // the map where elements are going to be merged into
IMapRootNodePtr _baseRoot; // a common ancestor of the two maps
IMapRootNodePtr _sourceRoot; // the map to be merged
IMapRootNodePtr _targetRoot; // the map where elements are going to be merged into

private:
// Volatile data only needed to construct the actions
Expand All @@ -34,8 +34,10 @@ class ThreeWayMergeOperation :
public:
using Ptr = std::shared_ptr<ThreeWayMergeOperation>;

ThreeWayMergeOperation(const scene::IMapRootNodePtr& baseRoot,
const scene::IMapRootNodePtr& sourceRoot, const scene::IMapRootNodePtr& targetRoot);
ThreeWayMergeOperation(const IMapRootNodePtr& baseRoot,
const IMapRootNodePtr& sourceRoot, const IMapRootNodePtr& targetRoot);

virtual ~ThreeWayMergeOperation();

// Creates the merge operation from the given comparison results.
// The operation will apply the missing changes in the source map to the target map
Expand All @@ -44,6 +46,16 @@ class ThreeWayMergeOperation :
void setMergeSelectionGroups(bool enabled) override;
void setMergeLayers(bool enabled) override;

const IMapRootNodePtr& getSourceRoot() const
{
return _sourceRoot;
}

const IMapRootNodePtr& getTargetRoot() const
{
return _targetRoot;
}

private:
void processEntityDifferences(const std::list<ComparisonResult::EntityDifference>& sourceDiffs,
const std::list<ComparisonResult::EntityDifference>& targetDiffs);
Expand Down
67 changes: 66 additions & 1 deletion test/MapMerging.cpp
Expand Up @@ -11,6 +11,7 @@
#include "scenelib.h"
#include "scene/merge/GraphComparer.h"
#include "scene/merge/MergeOperation.h"
#include "scene/merge/ThreeWayMergeOperation.h"
#include "scene/merge/SelectionGroupMerger.h"
#include "scene/merge/LayerMerger.h"

Expand All @@ -20,6 +21,7 @@ namespace test
using MapMergeTest = RadiantTest;
using SelectionGroupMergeTest = RadiantTest;
using LayerMergeTest = RadiantTest;
using ThreeWayMergeTest = RadiantTest;

TEST_F(MapMergeTest, BrushFingerprint)
{
Expand Down Expand Up @@ -380,7 +382,7 @@ TEST_F(MapMergeTest, DetectChildPrimitiveChanges)
}

template<typename T>
std::shared_ptr<T> findAction(const MergeOperation::Ptr& operation, const std::function<bool(const std::shared_ptr<T>&)>& predicate)
std::shared_ptr<T> findAction(const IMergeOperation::Ptr& operation, const std::function<bool(const std::shared_ptr<T>&)>& predicate)
{
std::shared_ptr<T> foundAction;

Expand Down Expand Up @@ -1756,4 +1758,67 @@ TEST_F(LayerMergeTest, MergeLayersFlagNotSet)
EXPECT_FALSE(merger->getChangeLog().empty());
}

// Map changelog of source and target against their base, used in several test cases below:
//
// Source:
// - light_2 has been added
// - brush 16 added to worldspawn
// - brush_8 in func_static_8 retextured to brush 9
// - entity expandable got a new spawnarg: "source_spawnarg" => "source_value"
// - entity expandable got a spawnarg removed: "extra1"
// - entity expandable got a spawnarg modified: "extra3" => "extra3_changed"
// - both brush_6 have been deleted from worldspawn
// - func_static_5 had two brush_5 added (were part of worldspawn before)
// - brush_11 got moved to the left
//
// Target Map:
// - light_3 has been added
// - brush_17 been added to worldspawn
// - brush_7 in func_static_7 retextured to brush 9
// - entity expandable got a new spawnarg: "target_spawnarg" => "target_value"
// - entity expandable got a spawnarg removed: "extra2"
// - entity expandable got a spawnarg modified: "origin" => "-100 350 32"
// - both brush_4 have been deleted from worldspawn
// - func_static_3 had two brush_3 added (were part of worldspawn before)
// - brush_12 got moved to the left
ThreeWayMergeOperation::Ptr setupThreeWayMergeOperation(const std::string& basePath, const std::string& targetPath, const std::string& sourcePath)
{
auto baseResource = GlobalMapResourceManager().createFromPath(basePath);
EXPECT_TRUE(baseResource->load()) << "Test map not found: " << basePath;

auto targetResource = GlobalMapResourceManager().createFromPath(targetPath);
EXPECT_TRUE(targetResource->load()) << "Test map not found: " << targetPath;

auto sourceResource = GlobalMapResourceManager().createFromPath(sourcePath);
EXPECT_TRUE(sourceResource->load()) << "Test map not found: " << sourcePath;

auto baseToSource = GraphComparer::Compare(sourceResource->getRootNode(), baseResource->getRootNode());
auto baseToTarget = GraphComparer::Compare(targetResource->getRootNode(), baseResource->getRootNode());

return ThreeWayMergeOperation::CreateFromComparisonResults(*baseToSource, *baseToTarget);
}

TEST_F(ThreeWayMergeTest, IndependentEntityAddition)
{
auto operation = setupThreeWayMergeOperation("maps/threeway_merge_base.mapx", "maps/threeway_merge_target_1.mapx", "maps/threeway_merge_source_1.mapx");

// light_2 must be added to target
auto action = findAction<AddEntityAction>(operation, [](const std::shared_ptr<AddEntityAction>& action)
{
auto sourceEntity = Node_getEntity(action->getSourceNodeToAdd());
return sourceEntity->getKeyValue("name") == "light_2";
});

EXPECT_TRUE(action) << "No merge action found for missing entity";

// Check pre-requisites and apply the action
auto entityNode = algorithm::getEntityByName(operation->getTargetRoot(), "light_2");
EXPECT_FALSE(entityNode);

action->applyChanges();

entityNode = algorithm::getEntityByName(operation->getTargetRoot(), "light_2");
EXPECT_TRUE(entityNode);
}

}

0 comments on commit 39f57ac

Please sign in to comment.