From d98874bd08a03826933c907dd0edc3f80bd6c45a Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 20 Oct 2018 15:55:46 -0700 Subject: [PATCH] Improve testability factoring of env_universal_t --- src/env.h | 4 +-- src/env_universal_common.cpp | 55 ++++++++++++++---------------------- src/env_universal_common.h | 3 ++ src/fish_tests.cpp | 39 +++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/env.h b/src/env.h index 114a91cbe..30fb4ab0d 100644 --- a/src/env.h +++ b/src/env.h @@ -127,8 +127,8 @@ class env_var_t { env_var_t &operator=(const env_var_t &var) = default; env_var_t &operator=(env_var_t &&) = default; - bool operator==(const env_var_t &var) const { return vals == var.vals; } - bool operator!=(const env_var_t &var) const { return vals != var.vals; } + bool operator==(const env_var_t &rhs) const { return vals == rhs.vals && flags == rhs.flags; } + bool operator!=(const env_var_t &rhs) const { return ! (*this == rhs); } }; /// Gets the variable with the specified name, or none() if it does not exist. diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 999b084cf..d0c257c54 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -391,45 +391,32 @@ bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t return result; } +/// Serialize the contents to a string. +std::string env_universal_t::serialize_with_vars(const var_table_t &vars) { + std::string storage; + std::string contents = SAVE_MSG; + for (const auto &kv : vars) { + // Append the entry. Note that append_file_entry may fail, but that only affects one + // variable; soldier on. + const wcstring &key = kv.first; + const env_var_t &var = kv.second; + append_file_entry( + var.exports() ? uvar_message_type_t::set_export : uvar_message_type_t::set, key, + encode_serialized(var.as_list()), &contents, &storage); + } + return contents; +} + /// Writes our state to the fd. path is provided only for error reporting. bool env_universal_t::write_to_fd(int fd, const wcstring &path) { ASSERT_IS_LOCKED(lock); assert(fd >= 0); bool success = true; - - // Stuff we output to fd. - std::string contents; - - // Temporary storage. - std::string storage; - - // Write the save message. If this fails, we don't bother complaining. - write_loop(fd, SAVE_MSG, strlen(SAVE_MSG)); - - var_table_t::const_iterator iter = vars.begin(); - while (iter != vars.end()) { - // Append the entry. Note that append_file_entry may fail, but that only affects one - // variable; soldier on. - const wcstring &key = iter->first; - const env_var_t &var = iter->second; - append_file_entry( - var.exports() ? uvar_message_type_t::set_export : uvar_message_type_t::set, key, - encode_serialized(var.as_list()), &contents, &storage); - - // Go to next. - ++iter; - - // Flush if this is the last iteration or we exceed a page. - if (iter == vars.end() || contents.size() >= 4096) { - if (write_loop(fd, contents.data(), contents.size()) < 0) { - const char *error = strerror(errno); - debug(0, _(L"Unable to write to universal variables file '%ls': %s"), path.c_str(), - error); - success = false; - break; - } - contents.clear(); - } + std::string contents = serialize_with_vars(vars); + if (write_loop(fd, contents.data(), contents.size()) < 0) { + const char *error = strerror(errno); + debug(0, _(L"Unable to write to universal variables file '%ls': %s"), path.c_str(), error); + success = false; } // Since we just wrote out this file, it matches our internal state; pretend we read from it. diff --git a/src/env_universal_common.h b/src/env_universal_common.h index daebe7a2c..263eb297c 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -109,6 +109,9 @@ class env_universal_t { /// Guess a file format. Exposed for testing only. static uvar_format_t format_for_contents(const std::string &s); + + /// Serialize a variable list. Exposed for testing only. + static std::string serialize_with_vars(const var_table_t &vars); }; /// The "universal notifier" is an object responsible for broadcasting and receiving universal diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index ce1282a9a..7ccf73bdc 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2888,6 +2888,35 @@ static void test_universal() { (void)system("rm -Rf test/fish_uvars_test/"); } +static void test_universal_output() { + say(L"Testing universal variable output"); + var_table_t vars; + vars[L"varA"] = env_var_t(wcstring_list_t{L"ValA1", L"ValA2"}, 0); + vars[L"varB"] = env_var_t(wcstring_list_t{L"ValB1"}, env_var_t::flag_export); + std::string text = env_universal_t::serialize_with_vars(vars); + const char *expected = + "# This file contains fish universal variable definitions.\n" + "SET varA:ValA1\\x1eValA2\n" + "SET_EXPORT varB:ValB1\n"; + do_test(text == expected); +} + +static void test_universal_parsing() { + say(L"Testing universal variable parsing"); + const char *input = + "# This file contains fish universal variable definitions.\n" + "SET varA:ValA1\\x1eValA2\n" + "SET_EXPORT varB:ValB1\n"; + + var_table_t vars; + vars[L"varA"] = env_var_t(wcstring_list_t{L"ValA1", L"ValA2"}, 0); + vars[L"varB"] = env_var_t(wcstring_list_t{L"ValB1"}, env_var_t::flag_export); + + var_table_t parsed_vars; + env_universal_t::populate_variables(input, &parsed_vars); + do_test(vars == parsed_vars); +} + static bool callback_data_less_than(const callback_data_t &a, const callback_data_t &b) { return a.key < b.key; } @@ -4602,6 +4631,14 @@ static void test_timezone_env_vars() { static void test_env_vars() { test_timezone_env_vars(); // TODO: Add tests for the locale and ncurses vars. + + env_var_t v1 = {L"abc", env_var_t::flag_export}; + env_var_t v2 = {wcstring_list_t{L"abc"}, env_var_t::flag_export}; + env_var_t v3 = {wcstring_list_t{L"abc"}, 0}; + env_var_t v4 = {wcstring_list_t{L"abc", L"def"}, env_var_t::flag_export}; + do_test(v1 == v2 && ! (v1 != v2)); + do_test(v1 != v3 && ! (v1 == v3)); + do_test(v1 != v4 && ! (v1 == v4)); } static void test_illegal_command_exit_code() { @@ -4844,6 +4881,8 @@ int main(int argc, char **argv) { if (should_test_function("input")) test_input(); if (should_test_function("line_iterator")) test_line_iterator(); if (should_test_function("universal")) test_universal(); + if (should_test_function("universal")) test_universal_output(); + if (should_test_function("universal")) test_universal_parsing(); if (should_test_function("universal")) test_universal_callbacks(); if (should_test_function("universal")) test_universal_formats(); if (should_test_function("notifiers")) test_universal_notifiers();