From 5f08ff5bb57699974c5aa06160471d021df94aa7 Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Tue, 28 Apr 2020 17:48:58 +0300 Subject: [PATCH 1/3] (maint) Move commit rake task from Rakefile to its own file. (#475) * (maint) Move commit rake task from Rakefile to its own file. * (maint) Fix rubocop. * (maint) Move commit check in separate job. --- .travis.yml | 4 ++++ Rakefile | 30 ------------------------------ tasks/commits.rake | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 30 deletions(-) create mode 100644 tasks/commits.rake diff --git a/.travis.yml b/.travis.yml index 3dcdce2b3..e0fc383aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,10 @@ jobs: script: - bundle exec rubocop - bundle exec rake spec + + - rvm: 2.6 + env: COMMIT_CHECK=true + script: - bundle exec rake commits - rvm: 2.5 diff --git a/Rakefile b/Rakefile index e7250afad..a80e7d6be 100644 --- a/Rakefile +++ b/Rakefile @@ -8,36 +8,6 @@ Dir.glob(File.join('tasks/**/*.rake')).each { |file| load file } task default: :spec -desc 'verify that commit messages match CONTRIBUTING.md requirements' -task(:commits) do - # This rake task looks at the summary from every commit from this branch not - # in the branch targeted for a PR. This is accomplished by using the - # TRAVIS_COMMIT_RANGE environment variable, which is present in travis CI and - # populated with the range of commits the PR contains. If not available, this - # falls back to `master..HEAD` as a next best bet as `master` is unlikely to - # ever be absent. - commit_range = ENV['TRAVIS_COMMIT_RANGE'].nil? ? 'master..HEAD' : ENV['TRAVIS_COMMIT_RANGE'].sub(/\.\.\./, '..') - puts "Checking commits #{commit_range}" - `git log --no-merges --pretty=%s #{commit_range}`.each_line do |commit_summary| - # This regex tests for the currently supported commit summary tokens: maint, doc, gem, or fact-. - # The exception tries to explain it in more full. - if /^\((maint|doc|docs|gem|fact-\d+)\)|revert/i.match(commit_summary).nil? - raise "\n\n\n\tThis commit summary didn't match CONTRIBUTING.md guidelines:\n" \ - "\n\t\t#{commit_summary}\n" \ - "\tThe commit summary (i.e. the first line of the commit message) should start with one of:\n" \ - "\t\t(FACT-) # this is most common and should be a ticket at tickets.puppet.com\n" \ - "\t\t(docs)\n" \ - "\t\t(docs)(DOCUMENT-)\n" \ - "\t\t(maint)\n" \ - "\t\t(gem)\n" \ - "\n\tThis test for the commit summary is case-insensitive.\n\n\n" - else - puts commit_summary.to_s - end - puts '...passed' - end -end - def retrieve_from_keyboard return unless ARGV =~ /changelog/ diff --git a/tasks/commits.rake b/tasks/commits.rake new file mode 100644 index 000000000..2192b2e20 --- /dev/null +++ b/tasks/commits.rake @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +desc 'verify that commit messages match CONTRIBUTING.md requirements' +task(:commits) do + # This rake task looks at the summary from every commit from this branch not + # in the branch targeted for a PR. This is accomplished by using the + # TRAVIS_COMMIT_RANGE environment variable, which is present in travis CI and + # populated with the range of commits the PR contains. If not available, this + # falls back to `master..HEAD` as a next best bet as `master` is unlikely to + # ever be absent. + commit_range = ENV['TRAVIS_COMMIT_RANGE'].nil? ? 'master..HEAD' : ENV['TRAVIS_COMMIT_RANGE'].sub(/\.\.\./, '..') + puts "Checking commits #{commit_range}" + `git log --no-merges --pretty=%s #{commit_range}`.each_line do |commit_summary| + # This regex tests for the currently supported commit summary tokens: maint, doc, gem, or fact-. + # The exception tries to explain it in more full. + if /^\((maint|doc|docs|gem|fact-\d+)\)|revert/i.match(commit_summary).nil? + raise "\n\n\n\tThis commit summary didn't match CONTRIBUTING.md guidelines:\n" \ + "\n\t\t#{commit_summary}\n" \ + "\tThe commit summary (i.e. the first line of the commit message) should start with one of:\n" \ + "\t\t(FACT-) # this is most common and should be a ticket at tickets.puppet.com\n" \ + "\t\t(docs)\n" \ + "\t\t(docs)(DOCUMENT-)\n" \ + "\t\t(maint)\n" \ + "\t\t(gem)\n" \ + "\n\tThis test for the commit summary is case-insensitive.\n\n\n" + else + puts commit_summary.to_s + end + puts '...passed' + end +end From 70dde94eef29a3d7df6281c89b34cb54ca1e092f Mon Sep 17 00:00:00 2001 From: Gheorghe Popescu Date: Tue, 28 Apr 2020 17:58:01 +0300 Subject: [PATCH 2/3] (maint) remove RSpec format documentation (#476) --- .rspec | 1 - 1 file changed, 1 deletion(-) diff --git a/.rspec b/.rspec index 34c5164d9..83e16f804 100644 --- a/.rspec +++ b/.rspec @@ -1,3 +1,2 @@ ---format documentation --color --require spec_helper From af87c828f30a15f84e3dec366a4c3a0ae38426ec Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Tue, 28 Apr 2020 18:33:28 +0300 Subject: [PATCH 3/3] (FACT-2585) Mountpoints fact returns ASCI-8BIT instead of UTF-8 in some cases (#472) --- lib/resolvers/utils/filesystem_helper.rb | 42 ++++++++++++------- .../resolvers/mountpoints_resolver_spec.rb | 8 ++-- .../utils/macosx/filesystem_helper_spec.rb | 31 ++++++++++++++ 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/lib/resolvers/utils/filesystem_helper.rb b/lib/resolvers/utils/filesystem_helper.rb index f52f514d4..b5911f203 100644 --- a/lib/resolvers/utils/filesystem_helper.rb +++ b/lib/resolvers/utils/filesystem_helper.rb @@ -5,24 +5,36 @@ module FilesystemHelper MOUNT_KEYS = %i[device filesystem path options available available_bytes size size_bytes used used_bytes capacity].freeze + class << self + def read_mountpoints + require 'sys/filesystem' + force_utf(Sys::Filesystem.mounts) + end - def self.read_mountpoints - require 'sys/filesystem' - Sys::Filesystem.mounts - end + def read_mountpoint_stats(path) + require 'sys/filesystem' + Sys::Filesystem.stat(path) + end - def self.read_mountpoint_stats(path) - require 'sys/filesystem' - Sys::Filesystem.stat(path) - end + def compute_capacity(used, total) + if used == total + '100%' + elsif used.positive? + "#{format('%.2f', 100.0 * used.to_f / total.to_f)}%" + else + '0%' + end + end + + private - def self.compute_capacity(used, total) - if used == total - '100%' - elsif used.positive? - "#{format('%.2f', 100.0 * used.to_f / total.to_f)}%" - else - '0%' + def force_utf(mounts) + mounts.each do |mount| + mount.name.force_encoding('UTF-8') + mount.mount_type.force_encoding('UTF-8') + mount.mount_point.force_encoding('UTF-8') + mount.options.force_encoding('UTF-8') + end end end end diff --git a/spec/facter/resolvers/mountpoints_resolver_spec.rb b/spec/facter/resolvers/mountpoints_resolver_spec.rb index f5cb103a6..1568965a9 100644 --- a/spec/facter/resolvers/mountpoints_resolver_spec.rb +++ b/spec/facter/resolvers/mountpoints_resolver_spec.rb @@ -37,8 +37,9 @@ allow(Facter::Util::FileHelper).to receive(:safe_read) .with('/proc/cmdline') .and_return(load_fixture('cmdline_root_device').read) - allow(Sys::Filesystem).to receive(:mounts).and_return([mount]) - allow(Sys::Filesystem).to receive(:stat).with(mount.mount_point).and_return(stat) + + allow(Facter::FilesystemHelper).to receive(:read_mountpoints).and_return([mount]) + allow(Facter::FilesystemHelper).to receive(:read_mountpoint_stats).with(mount.mount_point).and_return(stat) # mock sys/filesystem methods allow(stat).to receive(:bytes_total).and_return(stat.blocks * stat.fragment_size) @@ -55,7 +56,8 @@ end it 'drops automounts and non-tmpfs mounts under /proc or /sys' do - allow(Sys::Filesystem).to receive(:mounts).and_return(ignored_mounts) + allow(Facter::FilesystemHelper).to receive(:read_mountpoints).and_return(ignored_mounts) + result = Facter::Resolvers::Linux::Mountpoints.resolve(:mountpoints) expect(result).to be_empty end diff --git a/spec/facter/resolvers/utils/macosx/filesystem_helper_spec.rb b/spec/facter/resolvers/utils/macosx/filesystem_helper_spec.rb index 91e73a7f3..2e32bcab3 100644 --- a/spec/facter/resolvers/utils/macosx/filesystem_helper_spec.rb +++ b/spec/facter/resolvers/utils/macosx/filesystem_helper_spec.rb @@ -17,4 +17,35 @@ expect(capacity).to eq('4.21%') end end + + describe '#read_mountpoints' do + before do + mount = OpenStruct.new + mount.name = +'test_name'.encode('ASCII-8BIT') + mount.mount_type = +'test_type'.encode('ASCII-8BIT') + mount.mount_point = +'test_mount_point'.encode('ASCII-8BIT') + mount.options = +'test_options'.encode('ASCII-8BIT') + + mounts = [mount] + allow(Sys::Filesystem).to receive(:mounts).and_return(mounts) + end + + let(:mount_points) { Facter::FilesystemHelper.read_mountpoints } + + it 'converts name from ASCII-8BIT to UTF-8' do + expect(mount_points.first.name.encoding.name). to eq('UTF-8') + end + + it 'converts mount_type from ASCII-8BIT to UTF-8' do + expect(mount_points.first.mount_type.encoding.name). to eq('UTF-8') + end + + it 'converts mount_point from ASCII-8BIT to UTF-8' do + expect(mount_points.first.mount_point.encoding.name). to eq('UTF-8') + end + + it 'converts options from ASCII-8BIT to UTF-8' do + expect(mount_points.first.options.encoding.name). to eq('UTF-8') + end + end end