Make the MRU tests significantly less worthless

Originally committed to SVN as r6008.

aegisub/libaegisub/common/hotkey.cpp

@@ -21,6 +21,7 @@
 #include "../config.h"
 
 #ifndef LAGI_PRE
+#include <algorithm>
 #include <cmath>
 #include <memory>
 #endif
@@ -68,9 +69,7 @@ Hotkey::Hotkey(const std::string &file, const std::string &default_config)
 {
 	LOG_D("hotkey/init") << "Generating hotkeys.";
 
-	json::UnknownElement hotkey_root = agi::json_util::file(config_file, default_config);
+	json::Object object(agi::json_util::file(config_file, default_config));
-	json::Object const& object = hotkey_root;
-
 	for (json::Object::const_iterator index(object.begin()); index != object.end(); ++index)
 		BuildHotkey(index->first, index->second);
 }
@@ -160,12 +159,11 @@ void Hotkey::Flush() {
 	for (HotkeyMap::iterator index = str_map.begin(); index != str_map.end(); ++index) {
 		std::vector<std::string> const& combo_map(index->second.Get());
 
-		json::Array modifiers;
-		copy(combo_map.begin(), combo_map.end() - 1, std::back_inserter(modifiers));
-
 		json::Object hotkey;
-		hotkey["modifiers"] = modifiers;
 		hotkey["key"] = combo_map.back();
+		json::Array& modifiers = hotkey["modifiers"];
+
+		copy(combo_map.begin(), combo_map.end() - 1, std::back_inserter(modifiers));
 
 		json::Array& combo_array = root[index->second.Context()][index->second.CmdName()];
 		combo_array.push_back(hotkey);

aegisub/libaegisub/common/json.cpp

@@ -19,37 +19,35 @@
 /// @ingroup libaegisub io
 
 
-#ifdef LAGI_PRE
+#ifndef LAGI_PRE
 #include <fstream>
-#include <strstream>
+#include <sstream>
 #endif
 
 #include "libaegisub/access.h"
 #include "libaegisub/io.h"
 #include "libaegisub/json.h"
 #include "libaegisub/log.h"
+#include "libaegisub/scoped_ptr.h"
 
 
 namespace agi {
 	namespace json_util {
 
 json::UnknownElement parse(std::istream *stream) {
-	json::UnknownElement root;
-
 	try {
+		agi::scoped_ptr<std::istream> stream_deleter(stream);
+
+		json::UnknownElement root;
 		json::Reader::Read(root, *stream);
+		return root;
 	} catch (json::Reader::ParseException& e) {
 		LOG_E("json/parse") << "json::ParseException: " << e.what() << ", Line/offset: " << e.m_locTokenBegin.m_nLine + 1 << '/' << e.m_locTokenBegin.m_nLineOffset + 1;
-		delete stream;
 		throw;
 	} catch (json::Exception& e) {
 		LOG_E("json/parse") << "json::Exception: " << e.what();
-		delete stream;
 		throw;
 	}
-
-	delete stream;
-	return root;
 }
 
 json::UnknownElement file(const std::string &file) {

aegisub/libaegisub/common/log.cpp

@@ -67,18 +67,14 @@ LogSink::LogSink(const std::string& dir_log): dir_log(dir_log) {
 	util::time_log(time_start);
 }
 
-/// @todo The log files need to be trimed after N amount.
+/// @todo The log files need to be trimmed after N amount.
 LogSink::~LogSink() {
 	json::Object root;
-	json::Array array;
+	json::Array &array = root["log"];
 
 	agi_timeval time_close;
 	util::time_log(time_close);
 
-	std::stringstream str;
-	str << dir_log << time_start.tv_sec << ".json";
-	io::Save file(str.str());
-
 	for (unsigned int i=0; i < sink.size(); i++) {
 		json::Object entry;
 		entry["sec"]      = sink[i]->tv.tv_sec;
@@ -93,21 +89,17 @@ LogSink::~LogSink() {
 		array.push_back(entry);
 	}
 
-	root["timeval"] = json::Object();
+	json::Array &timeval_open = root["timeval"]["open"];
-
-	json::Array timeval_open;
 	timeval_open.push_back(time_start.tv_sec);
 	timeval_open.push_back(time_start.tv_usec);
-	root["timeval"]["open"] = timeval_open;
 
-	json::Array timeval_close;
+	json::Array &timeval_close = root["timeval"]["close"];
 	timeval_close.push_back(time_close.tv_sec);
 	timeval_close.push_back(time_close.tv_usec);
-	root["timeval"]["close"] = timeval_close;
 
-	root["log"] = array;
+	std::stringstream str;
-
+	str << dir_log << time_start.tv_sec << ".json";
-	json::Writer::Write(root, file.Get());
+	json::Writer::Write(root, io::Save(str.str()).Get());
 
 	agi::util::delete_clear(sink);
 }

aegisub/libaegisub/common/mru.cpp

@@ -31,12 +31,9 @@ namespace agi {
 MRUManager::MRUManager(const std::string &config, const std::string &default_config): config_name(config) {
 	LOG_D("agi/mru") << "Loading MRU List";
 
-	json::Object root_new = json_util::file(config, default_config);
+	json::Object root(json_util::file(config, default_config));
-
+	for (json::Object::const_iterator it(root.begin()); it != root.end(); ++it)
-	json::Object::const_iterator index_object(root_new.begin()), index_objectEnd(root_new.end());
+		Load(it->first, it->second);
-
-	for (; index_object != index_objectEnd; ++index_object)
-		Load(index_object->first, index_object->second);
 }
 
 

aegisub/src/toolbar.cpp

@@ -29,6 +29,7 @@
 #include "main.h"
 
 #ifndef AGI_PRE
+#include <sstream>
 #include <vector>
 
 #include <wx/frame.h>
@@ -104,7 +105,7 @@ namespace {
 
 			int icon_size = OPT_GET("App/Toolbar Icon Size")->GetInt();
 
-			json::Array arr = it->second;
+			json::Array const& arr = it->second;
 			commands.reserve(arr.size());
 			bool needs_onidle = false;
 

aegisub/tests/libaegisub_mru.cpp

@@ -27,34 +27,38 @@ protected:
 	std::string default_mru;
 	std::string conf_ok;
 
-	virtual void SetUp() {
+	void SetUp() {
-		default_mru = "{\"Valid_Int\" : []}";
+		default_mru = "{\"Valid\" : []}";
 		conf_ok = "./data/mru_ok.json";
 	}
 };
 
 
 TEST_F(lagi_mru, MRUConstructFromFile) {
-	EXPECT_NO_THROW(agi::MRUManager mru(conf_ok, default_mru));
+	ASSERT_NO_THROW(agi::MRUManager mru(conf_ok, default_mru));
+	agi::MRUManager mru(conf_ok, default_mru);
+	agi::MRUManager::MRUListMap::const_iterator entry = mru.Get("Valid")->begin();
+	EXPECT_STREQ("Entry One", (*entry++).c_str());
+	EXPECT_STREQ("Entry Two", (*entry++).c_str());
+	EXPECT_TRUE(mru.Get("Valid")->end() == entry);
 }
 
 TEST_F(lagi_mru, MRUConstructFromString) {
 	util::remove("data/mru_tmp");
-
+	agi::MRUManager mru("data/mru_tmp", default_mru);
-	const std::string nonexistent("data/mru_tmp");
-	agi::MRUManager mru(nonexistent, default_mru);
 }
 
-
 TEST_F(lagi_mru, MRUConstructInvalid) {
 	util::copy("data/mru_invalid.json", "data/mru_tmp");
-	EXPECT_ANY_THROW(agi::MRUManager("data/mru_tmp", default_mru));
+	agi::MRUManager mru("data/mru_tmp", default_mru);
+	EXPECT_TRUE(mru.Get("Invalid")->empty());
 }
 
 TEST_F(lagi_mru, MRUEntryAdd) {
 	util::copy("data/mru_ok.json", "data/mru_tmp");
 	agi::MRUManager mru("data/mru_tmp", default_mru);
 	EXPECT_NO_THROW(mru.Add("Valid", "/path/to/file"));
+	EXPECT_STREQ("/path/to/file", mru.Get("Valid")->front().c_str());
 }
 
 TEST_F(lagi_mru, MRUEntryRemove) {
@@ -62,12 +66,14 @@ TEST_F(lagi_mru, MRUEntryRemove) {
 	agi::MRUManager mru("data/mru_tmp", default_mru);
 	EXPECT_NO_THROW(mru.Add("Valid", "/path/to/file"));
 	EXPECT_NO_THROW(mru.Remove("Valid", "/path/to/file"));
+	EXPECT_STRNE("/path/to/file", mru.Get("Valid")->front().c_str());
 }
 
 TEST_F(lagi_mru, MRUKeyInvalid) {
 	util::copy("data/mru_ok.json", "data/mru_tmp");
 	agi::MRUManager mru("data/mru_tmp", default_mru);
 	EXPECT_THROW(mru.Add("Invalid", "/path/to/file"), agi::MRUErrorInvalidKey);
+	EXPECT_THROW(mru.Get("Invalid"), agi::MRUErrorInvalidKey);
 }
 
 TEST_F(lagi_mru, MRUKeyValid) {
@@ -77,7 +83,7 @@ TEST_F(lagi_mru, MRUKeyValid) {
 }
 
 TEST_F(lagi_mru, MRUAddSeveral) {
-	util::copy("data/mru_ok.json", "data/mru_tmp");
+	util::remove("data/mru_tmp");
 	agi::MRUManager mru("data/mru_tmp", default_mru);
 
 	EXPECT_NO_THROW(mru.Add("Valid", "/file/1"));
@@ -87,15 +93,12 @@ TEST_F(lagi_mru, MRUAddSeveral) {
 	EXPECT_NO_THROW(mru.Add("Valid", "/file/1"));
 	EXPECT_NO_THROW(mru.Add("Valid", "/file/3"));
 
-	agi::MRUManager::MRUListMap::const_iterator entry = mru.Get("Valid")->begin();
+	EXPECT_STREQ("/file/3", mru.GetEntry("Valid", 0).c_str());
-	EXPECT_STREQ("/file/3", (*entry++).c_str());
+	EXPECT_STREQ("/file/1", mru.GetEntry("Valid", 1).c_str());
-	EXPECT_STREQ("/file/1", (*entry++).c_str());
+	EXPECT_STREQ("/file/2", mru.GetEntry("Valid", 2).c_str());
-	EXPECT_STREQ("/file/2", (*entry++).c_str());
+	EXPECT_THROW(mru.GetEntry("Valid", 3), agi::MRUErrorIndexOutOfRange);
-	EXPECT_TRUE(mru.Get("Valid")->end() == entry);
 }
 
-
-
 // Check to make sure an entry is really removed.  This was fixed in
 // r4347, the entry was being removed from a copy of the map internally.
 TEST_F(lagi_mru, MRUEntryRemove_r4347) {

aegisub/tests/setup.bat

@@ -18,9 +18,9 @@ icacls data\dir_access_denied /deny %USERNAME%:F
 mkdir data\dir_read_only
 icacls data\dir_read_only /deny %USERNAME%:W
 
-echo {"Valid" : []} > data/mru_ok.json
+echo {"Valid" : ["Entry One", "Entry Two"]} > data/mru_ok.json
 
-echo {"Invalid" : [1 3]} > data/mru_invalid.json
+echo {"Invalid" : [1, 3]} > data/mru_invalid.json
 
 echo > data/rename_me
 

aegisub/tests/setup.sh

@@ -20,9 +20,9 @@ chmod 000 data/dir_access_denied
 mkdir data/dir_read_only
 chmod 444 data/dir_read_only
 
-echo '{"Valid" : []}' > data/mru_ok.json
+echo '{"Valid" : ["Entry One", "Entry Two"]}' > data/mru_ok.json
 
-echo '{"Invalid" : [1 3]}' > data/mru_invalid.json
+echo '{"Invalid" : [1, 3]}' > data/mru_invalid.json
 
 touch data/rename_me