From f0ee7630ed8993ef80cd11b12330c78616819ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 11 Aug 2025 20:01:45 +0200 Subject: Merge specs checking CLI flags and subcommands are documented We had them duplicated, but with slightly different features: * The ones in `other/cli_man_pages.rb` enforced a specific structure to document CLI options, so were less likely to have false positives. * The ones in `quality_spec.rb` were able to check subcommands and their flags. This commit merges both and preserves the best of both. --- spec/bundler/other/cli_man_pages_spec.rb | 85 +++++++++++++++++++++----------- spec/bundler/quality_spec.rb | 49 ------------------ 2 files changed, 56 insertions(+), 78 deletions(-) diff --git a/spec/bundler/other/cli_man_pages_spec.rb b/spec/bundler/other/cli_man_pages_spec.rb index 84ffca14e6..6efd2904d6 100644 --- a/spec/bundler/other/cli_man_pages_spec.rb +++ b/spec/bundler/other/cli_man_pages_spec.rb @@ -1,49 +1,72 @@ # frozen_string_literal: true RSpec.describe "bundle commands" do - it "expects all commands to have a man page" do - Bundler::CLI.all_commands.each_key do |command_name| - next if command_name == "cli_help" + it "expects all commands to have all options and subcommands documented" do + check_commands!(Bundler::CLI) - expect(man_page(command_name)).to exist + Bundler::CLI.subcommand_classes.each_value do |klass| + check_commands!(klass) end end - it "expects all commands to have all options documented" do - Bundler::CLI.all_commands.each do |command_name, command| - next if command_name == "cli_help" + private + + def check_commands!(command_class) + command_class.commands.each do |command_name, command| + next if command.is_a?(Bundler::Thor::HiddenCommand) + + if command_class == Bundler::CLI + man_page = man_page(command_name) + expect(man_page).to exist + + check_options!(command, man_page) + else + man_page = man_page(command.ancestor_name) + expect(man_page).to exist - man_page_content = man_page(command_name).read + check_options!(command, man_page) + check_subcommand!(command_name, man_page) + end + end + end - command.options.each do |_, option| - aliases = option.aliases - formatted_aliases = aliases.sort.map {|name| "`#{name}`" }.join(", ") if aliases + def check_options!(command, man_page) + command.options.each do |_, option| + check_option!(option, man_page) + end + end - help = if option.type == :boolean - "* #{append_aliases("`#{option.switch_name}`", formatted_aliases)}:" - elsif option.enum - formatted_aliases = "`#{option.switch_name}`" if aliases.empty? && option.lazy_default - "* #{prepend_aliases(option.enum.sort.map {|enum| "`#{option.switch_name}=#{enum}`" }.join(", "), formatted_aliases)}:" - else - names = [option.switch_name, *aliases] - value = - case option.type - when :array then "" - when :numeric then "" - else option.name.upcase - end + def check_option!(option, man_page) + man_page_content = man_page.read - value = option.type != :numeric && option.lazy_default ? "[=#{value}]" : "=#{value}" + aliases = option.aliases + formatted_aliases = aliases.sort.map {|name| "`#{name}`" }.join(", ") if aliases - "* #{names.map {|name| "`#{name}#{value}`" }.join(", ")}:" + help = if option.type == :boolean + "* #{append_aliases("`#{option.switch_name}`", formatted_aliases)}:" + elsif option.enum + formatted_aliases = "`#{option.switch_name}`" if aliases.empty? && option.lazy_default + "* #{prepend_aliases(option.enum.sort.map {|enum| "`#{option.switch_name}=#{enum}`" }.join(", "), formatted_aliases)}:" + else + names = [option.switch_name, *aliases] + value = + case option.type + when :array then "" + when :numeric then "" + else option.name.upcase end - expect(man_page_content).to include(help) - end + value = option.type != :numeric && option.lazy_default ? "[=#{value}]" : "=#{value}" + + "* #{names.map {|name| "`#{name}#{value}`" }.join(", ")}:" end + + expect(man_page_content).to include(help) end - private + def check_subcommand!(name, man_page) + expect(man_page.read).to match(name) + end def append_aliases(text, aliases) return text if aliases.empty? @@ -57,6 +80,10 @@ RSpec.describe "bundle commands" do "#{aliases}, #{text}" end + def man_page_content(command_name) + man_page(command_name).read + end + def man_page(command_name) source_root.join("lib/bundler/man/bundle-#{command_name}.1.ronn") end diff --git a/spec/bundler/quality_spec.rb b/spec/bundler/quality_spec.rb index 18d8e20030..3e5a960a96 100644 --- a/spec/bundler/quality_spec.rb +++ b/spec/bundler/quality_spec.rb @@ -251,58 +251,9 @@ RSpec.describe "The library itself" do expect(lib_code).to eq(spec_code) end - it "documents all cli command options in their associated man pages" do - commands = normalize_commands_and_options(Bundler::CLI) - cli_and_man_pages_in_sync!(commands) - - Bundler::CLI.subcommand_classes.each do |_, klass| - subcommands = normalize_commands_and_options(klass) - - cli_and_man_pages_in_sync!(subcommands) - end - end - private def each_line(filename, &block) File.readlines(filename, encoding: "UTF-8").each_with_index(&block) end - - def normalize_commands_and_options(command_class) - commands = {} - - command_class.commands.each do |command_name, command| - next if command.is_a?(Bundler::Thor::HiddenCommand) - - key = command.ancestor_name || command_name - commands[key] ||= [] - # Verify that all subcommands are documented in the main command's man page. - commands[key] << command_name unless command_class == Bundler::CLI - - command.options.each do |_, option| - commands[key] << option.switch_name - end - end - - commands - end - - def cli_and_man_pages_in_sync!(commands) - commands.each do |command_name, opts| - man_page_path = man_tracked_files.find {|f| File.basename(f) == "bundle-#{command_name}.1.ronn" } - expect(man_page_path).to_not be_nil, "The command #{command_name} has no associated man page." - - next if opts.empty? - - man_page_content = File.read(man_page_path) - opts.each do |option_name| - error_msg = <<~EOM - The command #{command_name} has no mention of the option or subcommand `#{option_name}` in its man page. - Document the `#{option_name}` in the man page to discard this error. - EOM - - expect(man_page_content).to match(option_name), error_msg - end - end - end end -- cgit v1.2.3