Simplify/fix task dependency calculation. And tasks unit-tests.

10 files changed, 286 insertions, 52 deletions

diff --git a/ctor.cc b/ctor.cc
index 6dbb96d..2e63f77 100644
--- a/ctor.cc
+++ b/ctor.cc
@@ -21,6 +21,7 @@ BuildConfigurations ctorConfigs()
 				"src/task_ld.cc",
 				"src/task_so.cc",
 				"src/tasks.cc",
+				"src/unittest.cc",
 			},
 			.cxxflags = {
 				"-std=c++17",
diff --git a/src/task.cc b/src/task.cc
index 962a02b..4cdfa5e 100644
--- a/src/task.cc
+++ b/src/task.cc
@@ -3,17 +3,15 @@
 #include <unistd.h>
 #include <iostream>
 
-Task::Task(const BuildConfiguration& config,
-           const std::vector<std::string>& depends)
-	: dependsStr(depends)
-	, config(config)
+Task::Task(const BuildConfiguration& config)
+	: config(config)
 	, output_system(config.system)
 {
 }
 
 int Task::registerDepTasks(const std::list<std::shared_ptr<Task>>& tasks)
 {
-	for(auto const& depStr : dependsStr)
+	for(const auto& depStr : depends())
 	{
 		bool found{false};
 		for(const auto& task : tasks)
diff --git a/src/task.h b/src/task.h
index acbefb3..4461612 100644
--- a/src/task.h
+++ b/src/task.h
@@ -21,8 +21,7 @@ enum class State
 class Task
 {
 public:
-	Task(const BuildConfiguration& config,
-	     const std::vector<std::string>& depends = {});
+	Task(const BuildConfiguration& config);
 
 	int registerDepTasks(const std::list<std::shared_ptr<Task>>& tasks);
 
diff --git a/src/task_ar.cc b/src/task_ar.cc
index 980d8b4..51d609e 100644
--- a/src/task_ar.cc
+++ b/src/task_ar.cc
@@ -22,26 +22,13 @@ std::string readFile(const std::string &fileName)
 
 	return std::string(bytes.data(), fileSize);
 }
-
-std::vector<std::string> addPrefix(const std::vector<std::string>& lst,
-                                   const Settings& settings)
-{
-	std::vector<std::string> out;
-	for(const auto& item : lst)
-	{
-		std::filesystem::path file = settings.builddir;
-		file /= item;
-		out.push_back(file.string());
-	}
-	return out;
-}
 } // namespace ::
 
 TaskAR::TaskAR(const BuildConfiguration& config,
                const Settings& settings,
                const std::string& target,
                const std::vector<std::string>& objects)
-	: Task(config, addPrefix(config.depends, settings))
+	: Task(config)
 	, config(config)
 	, settings(settings)
 {
diff --git a/src/task_ld.cc b/src/task_ld.cc
index ec68190..4eb64f4 100644
--- a/src/task_ld.cc
+++ b/src/task_ld.cc
@@ -21,26 +21,13 @@ std::string readFile(const std::string &fileName)
 
 	return std::string(bytes.data(), fileSize);
 }
-
-std::vector<std::string> addPrefix(const std::vector<std::string>& lst,
-                                   const Settings& settings)
-{
-	std::vector<std::string> out;
-	for(const auto& item : lst)
-	{
-		std::filesystem::path file = settings.builddir;
-		file /= item;
-		out.push_back(file.string());
-	}
-	return out;
-}
 } // namespace ::
 
 TaskLD::TaskLD(const BuildConfiguration& config,
                const Settings& settings,
                const std::string& target,
                const std::vector<std::string>& objects)
-	: Task(config, addPrefix(config.depends, settings))
+	: Task(config)
 	, config(config)
 	, settings(settings)
 {
diff --git a/src/task_so.cc b/src/task_so.cc
index ca7883f..519085a 100644
--- a/src/task_so.cc
+++ b/src/task_so.cc
@@ -21,26 +21,13 @@ std::string readFile(const std::string &fileName)
 
 	return std::string(bytes.data(), fileSize);
 }
-
-std::vector<std::string> addPrefix(const std::vector<std::string>& lst,
-                                   const Settings& settings)
-{
-	std::vector<std::string> out;
-	for(const auto& item : lst)
-	{
-		std::filesystem::path file = settings.builddir;
-		file /= item;
-		out.push_back(file.string());
-	}
-	return out;
-}
 } // namespace ::
 
 TaskSO::TaskSO(const BuildConfiguration& config,
                const Settings& settings,
                const std::string& target,
                const std::vector<std::string>& objects)
-	: Task(config, addPrefix(config.depends, settings))
+	: Task(config)
 	, config(config)
 	, settings(settings)
 {
diff --git a/src/tasks.h b/src/tasks.h
index e5d1daf..c5b326e 100644
--- a/src/tasks.h
+++ b/src/tasks.h
@@ -17,9 +17,17 @@ struct Target
 	std::string path;
 };
 
+//! Get list of all registered targets
 const std::deque<Target>& getTargets(const Settings& settings);
 
+//! Returns next dirty task from the dirtyTasks list that has all its dependencies
+//! fulfilled.
+//! The returned task is removed from the dirty list.
+//! Return nullptr if no dirty task is ready.
 std::shared_ptr<Task> getNextTask(const std::list<std::shared_ptr<Task>>& allTasks,
                                   std::list<std::shared_ptr<Task>>& dirtyTasks);
+
+//! Get list of tasks filtered by name including each of their direct
+//! dependency tasks (ie. objects tasks from their sources).
 std::list<std::shared_ptr<Task>> getTasks(const Settings& settings,
                                           const std::vector<std::string> names = {});
diff --git a/test/ctor.cc b/test/ctor.cc
index aed3ee5..caa00ee 100644
--- a/test/ctor.cc
+++ b/test/ctor.cc
@@ -1,4 +1,4 @@
-#include "libctor.h"
+#include <libctor.h>
 
 namespace
 {
@@ -19,7 +19,21 @@ BuildConfigurations ctorTestConfigs()
 				"-I../src", "-Iuunit",
 				"-DOUTPUT=\"execute\"",
 			},
-		}
+		},
+		{
+			.type = TargetType::UnitTest,
+			.target = "tasks_test",
+			.sources = {
+				"tasks_test.cc",
+				"uunit/uunit.cc",
+			},
+			.depends = {"libctor.a"},
+			.cxxflags = {
+				"-std=c++17", "-O3", "-s", "-Wall", "-Werror",
+				"-I../src", "-Iuunit",
+				"-DOUTPUT=\"tasks\"",
+			},
+		},
 	};
 }
 }
diff --git a/test/execute_test.cc b/test/execute_test.cc
index e055757..9da18dc 100644
--- a/test/execute_test.cc
+++ b/test/execute_test.cc
@@ -1,6 +1,6 @@
 #include <uunit.h>
 
-#include "../src/execute.h"
+#include <execute.h>
 
 class ExecuteTest
 	: public uUnit
diff --git a/test/tasks_test.cc b/test/tasks_test.cc
new file mode 100644
index 0000000..d6515b8
--- /dev/null
+++ b/test/tasks_test.cc
@@ -0,0 +1,253 @@
+#include <uunit.h>
+
+#include <libctor.h>
+#include <tasks.h>
+#include <settings.h>
+
+namespace
+{
+BuildConfigurations ctorTestConfigs1()
+{
+	return
+	{
+		{
+			.target = "target1",
+			.sources = {"foo.cc", "bar.c"},
+		},
+		{
+			.target = "target2",
+		},
+	};
+}
+
+BuildConfigurations ctorTestConfigs2()
+{
+	return
+	{
+		{
+			.target = "target3",
+		},
+		{
+			.target = "target4",
+		},
+	};
+}
+}
+
+REG(ctorTestConfigs1);
+REG(ctorTestConfigs2);
+
+
+class TestTask
+	: public Task
+{
+public:
+	TestTask(const std::string& name, bool dirty,
+	         const std::vector<std::string>& deps = {})
+		: Task({})
+		, task_name(name)
+		, task_dirty(dirty)
+		, task_deps(deps)
+	{
+	}
+
+	std::string name() const override { return task_name; }
+	int clean() override { return 0; }
+	std::vector<std::string> depends() const override { return task_deps; }
+	std::string target() const override { return task_name; }
+	bool dirtyInner() override { return task_dirty; }
+
+private:
+	std::string task_name;
+	bool task_dirty;
+	std::vector<std::string> task_deps;
+};
+
+class TasksTest
+	: public uUnit
+{
+public:
+	TasksTest()
+	{
+		uTEST(TasksTest::getTargets_test);
+		uTEST(TasksTest::getTasks_test);
+		uTEST(TasksTest::getNextTask_test);
+	}
+
+	void getTargets_test()
+	{
+		using namespace std::string_literals;
+		Settings settings{};
+		const auto& targets = getTargets(settings);
+		uASSERT_EQUAL(4u, targets.size());
+
+		uASSERT_EQUAL("target1"s, targets[0].config.target);
+		uASSERT_EQUAL("target2"s, targets[1].config.target);
+		uASSERT_EQUAL("target3"s, targets[2].config.target);
+		uASSERT_EQUAL("target4"s, targets[3].config.target);
+
+		uASSERT_EQUAL("test"s, targets[0].path);
+		uASSERT_EQUAL("test"s, targets[1].path);
+		uASSERT_EQUAL("test"s, targets[2].path);
+		uASSERT_EQUAL("test"s, targets[3].path);
+	}
+
+	void getTasks_test()
+	{
+		using namespace std::string_literals;
+		Settings settings{};
+		{
+			auto tasks = getTasks(settings);
+			uASSERT_EQUAL(6u, tasks.size());
+			auto task = tasks.begin();
+			uASSERT_EQUAL("target1-foo_cc.o"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target1-bar_c.o"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target1"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target2"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target3"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target4"s, (*task)->target());
+		}
+		{
+			auto tasks = getTasks(settings, {"target1", "target3"});
+			uASSERT_EQUAL(4u, tasks.size());
+			auto task = tasks.begin();
+			uASSERT_EQUAL("target1-foo_cc.o"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target1-bar_c.o"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target1"s, (*task)->target());
+			task++;
+			uASSERT_EQUAL("target3"s, (*task)->target());
+		}
+		{
+			auto tasks = getTasks(settings, {"no-such-target"});
+			uASSERT_EQUAL(0u, tasks.size());
+		}
+	}
+
+	void getNextTask_test()
+	{
+		using namespace std::string_literals;
+		Settings settings{};
+
+		{ // Zero (Empty)
+			std::list<std::shared_ptr<Task>> allTasks;
+			std::list<std::shared_ptr<Task>> dirtyTasks;
+
+			for(auto& task : dirtyTasks)
+			{
+				uASSERT_EQUAL(0, task->registerDepTasks(allTasks));
+			}
+
+			uASSERT_EQUAL(nullptr, getNextTask(allTasks, dirtyTasks));
+		}
+
+		{ // Zero (One task, no dirty)
+			auto task1 = std::make_shared<TestTask>("task1", false);
+
+			std::list<std::shared_ptr<Task>> allTasks;
+			allTasks.push_back(task1);
+
+			std::list<std::shared_ptr<Task>> dirtyTasks;
+
+			for(auto& task : dirtyTasks)
+			{
+				uASSERT_EQUAL(0, task->registerDepTasks(allTasks));
+			}
+
+			uASSERT_EQUAL(nullptr, getNextTask(allTasks, dirtyTasks));
+		}
+
+		{ // One (One task, one dirty)
+			auto task1 = std::make_shared<TestTask>("task1", true);
+
+			std::list<std::shared_ptr<Task>> allTasks;
+			allTasks.push_back(task1);
+
+			std::list<std::shared_ptr<Task>> dirtyTasks;
+			dirtyTasks.push_back(task1);
+
+			for(auto& task : dirtyTasks)
+			{
+				uASSERT_EQUAL(0, task->registerDepTasks(allTasks));
+			}
+
+			uASSERT_EQUAL(task1, getNextTask(allTasks, dirtyTasks));
+			uASSERT_EQUAL(0u, dirtyTasks.size());
+		}
+
+		{ // One (Two tasks, one dirty)
+			auto task1 = std::make_shared<TestTask>("task1", false);
+			auto task2 = std::make_shared<TestTask>("task2", true);
+
+			std::list<std::shared_ptr<Task>> allTasks;
+			allTasks.push_back(task1);
+			allTasks.push_back(task2);
+
+			std::list<std::shared_ptr<Task>> dirtyTasks;
+			dirtyTasks.push_back(task2);
+
+			for(auto& task : dirtyTasks)
+			{
+				uASSERT_EQUAL(0, task->registerDepTasks(allTasks));
+			}
+
+			uASSERT_EQUAL(task2, getNextTask(allTasks, dirtyTasks));
+			uASSERT_EQUAL(0u, dirtyTasks.size());
+		}
+
+		{ // One (Two tasks, one dirty which depends on the other)
+			auto task1 = std::make_shared<TestTask>("task1", false);
+
+			std::vector<std::string> deps = {"task1"};
+			auto task2 = std::make_shared<TestTask>("task2", true, deps);
+
+			std::list<std::shared_ptr<Task>> allTasks;
+			allTasks.push_back(task1);
+			allTasks.push_back(task2);
+
+			std::list<std::shared_ptr<Task>> dirtyTasks;
+			dirtyTasks.push_back(task2);
+
+			for(auto& task : dirtyTasks)
+			{
+				uASSERT_EQUAL(0, task->registerDepTasks(allTasks));
+			}
+
+			uASSERT_EQUAL(task2, getNextTask(allTasks, dirtyTasks));
+			uASSERT_EQUAL(0u, dirtyTasks.size());
+		}
+
+		{ // One (Two tasks, Both dirty, one depends on the other)
+			auto task1 = std::make_shared<TestTask>("task1", true);
+
+			std::vector<std::string> deps = {"task1"};
+			auto task2 = std::make_shared<TestTask>("task2", true, deps);
+
+			std::list<std::shared_ptr<Task>> allTasks;
+			allTasks.push_back(task2);
+			allTasks.push_back(task1);
+
+			std::list<std::shared_ptr<Task>> dirtyTasks;
+			dirtyTasks.push_back(task2);
+			dirtyTasks.push_back(task1);
+
+			for(auto& task : dirtyTasks)
+			{
+				uASSERT_EQUAL(0, task->registerDepTasks(allTasks));
+			}
+
+			uASSERT_EQUAL(task1, getNextTask(allTasks, dirtyTasks));
+			uASSERT_EQUAL(1u, dirtyTasks.size());
+		}
+
+	}
+};
+
+// Registers the fixture into the 'registry'
+static TasksTest test;