From c1e52a05c5fba012a6858c9b1a333ffacb793bc4 Mon Sep 17 00:00:00 2001 From: "Marc A. Paradise" Date: Thu, 21 Mar 2019 08:25:34 -0400 Subject: WIP to correct a test that can cause all the rest of them to abort with successful test run Signed-off-by: Marc A. Paradise --- spec/unit/application/client_spec.rb | 55 +++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/spec/unit/application/client_spec.rb b/spec/unit/application/client_spec.rb index f971848c5b..c3ace13121 100644 --- a/spec/unit/application/client_spec.rb +++ b/spec/unit/application/client_spec.rb @@ -208,20 +208,49 @@ describe Chef::Application::Client, "reconfigure" do end end - describe "--recipe-url and --local-mode" do + describe "--recipe-url and --local-mode with --config PATH for isolated tests" do let(:archive) { double } let(:config_exists) { false } + let(:repo_path) { "the_path_to_the_repo" } + let(:config_path) { File.join(repo_path, ".chef/config.rb") } before do + # Note - We set this to ensure that the config file is loaded + # relative to a known location (repo_path), and use that to hook + # file operations below. This works within the reconfigure method itself, + # but it breaks down in the actual call chain: + # + # #reconfigure invokes super.reconfigure + # #super.reconfigure invokes Client.load_config_file + # Client.load_config_file invokes super.load_config_file, using + # Chef::WorkstationConfigLoader (for localmode) . + # WorkstationConfigLoader does not look at Config.repo_path when determining + # where the config file is located. + # + # There are two problesm with this: + # * This means that the config file is _not_ mocked for these reconfigure tests + # and we run with whatever is found locally in the usual paths. + # * If we set an expectation assuming those mocks are correct + # (such as the test for "when there is new config" below where we check `not receive`) + # the expectation fails and raises an exception as soon as we call the + # function we shouldn't be - and Applications top-level error handling will kick in + # + # By providing an explicit config option, we ensure that + # the remaining mocks will hit the right path and be valid. + + # Still need to do this because Client#reconfigure expects it. + # TODO - this is a bug? allow(Chef::Config).to receive(:chef_repo_path).and_return("the_path_to_the_repo") + + allow(IO).to receive(:read).with(config_path).and_return("new_config") + allow(File).to receive(:file?).with(config_path).and_return(config_exists) allow(FileUtils).to receive(:rm_rf) allow(FileUtils).to receive(:mkdir_p) - allow(app).to receive(:fetch_recipe_tarball) + allow(Chef::Config).to receive(:from_string) allow(Mixlib::Archive).to receive(:new).and_return(archive) + allow(archive).to receive(:extract) - allow(Chef::Config).to receive(:from_string) - allow(IO).to receive(:read).with(File.join("the_path_to_the_repo", ".chef/config.rb")).and_return("new_config") - allow(File).to receive(:file?).with(File.join("the_path_to_the_repo", ".chef/config.rb")).and_return(config_exists) + allow(app).to receive(:fetch_recipe_tarball) end context "local mode not set" do @@ -234,17 +263,17 @@ describe Chef::Application::Client, "reconfigure" do context "local mode set" do before do - ARGV.replace(["--local-mode", "--recipe-url=test_url"]) + ARGV.replace(["--local-mode", "--recipe-url=test_url", "--config", config_path]) end context "--delete-entire-chef-repo" do before do - ARGV.replace(["--local-mode", "--recipe-url=test_url", "--delete-entire-chef-repo"]) + ARGV.replace(["--local-mode", "--recipe-url=test_url", "--delete-entire-chef-repo", "--config", config_path]) end it "deletes the repo" do expect(FileUtils).to receive(:rm_rf) - .with("the_path_to_the_repo", secure: true) + .with(repo_path, secure: true) app.reconfigure end @@ -264,25 +293,25 @@ describe Chef::Application::Client, "reconfigure" do it "makes the repo path" do expect(FileUtils).to receive(:mkdir_p) - .with("the_path_to_the_repo") + .with(repo_path) app.reconfigure end it "fetches the tarball" do expect(app).to receive(:fetch_recipe_tarball) - .with("test_url", File.join("the_path_to_the_repo", "recipes.tgz")) + .with("test_url", File.join(repo_path, "recipes.tgz")) app.reconfigure end it "extracts the archive" do expect(Mixlib::Archive).to receive(:new) - .with(File.join("the_path_to_the_repo", "recipes.tgz")) + .with(File.join(repo_path, "recipes.tgz")) .and_return(archive) expect(archive).to receive(:extract) - .with("the_path_to_the_repo", perms: false, ignore: /^\.$/) + .with(repo_path, perms: false, ignore: /^\.$/) app.reconfigure end @@ -294,7 +323,7 @@ describe Chef::Application::Client, "reconfigure" do expect(Chef::Config).to receive(:from_string) .with( "new_config", - File.join("the_path_to_the_repo", ".chef/config.rb") + File.join(repo_path, ".chef/config.rb") ) app.reconfigure -- cgit v1.2.1