wiki.ohea.xyz/gollum
10
0
Fork 0
Code Issues Pull Requests 1 Packages Projects Releases Wiki Activity

Switch from TestUnit to Minitest (#1805)

Browse Source 
* Use `Minitest::Test`

`Test::Unit` is deprecated, and we can switch to `Minitest::Test` with
almost no side effects.

This commit does all of the work required to make Minitest tests run
with Gollum's existing test helpers.

* Change Minitest output format

- The `DefaultReporter` seems to have cleaner output than what we had
  before.
- `color: true` ensures things are colorized. It's pretty nice.

* Tweak test formatting; fix order-dependent failure

The order-dependent failure has been been commented in code.

After manually bisecting the test suite, I was able to determine that
the template cascade tests were leaving the `@@template_priority_path`
set to an overridden value in tests run afterward.

* Tweak setting initialization

I could not see a meaningful reason behind calling `forbid` so early in
the settings initialization block. But moving the `forbid` call until
later resolved this issue.

In the real world, I don't see how this would cause issues. But I found
that calling `forbid` when `wiki_options[:allow_editing]` was set to
false caused order-dependent test errors where Sprockets would end up
being badly configured and left without an initialized
`Sprockets::Environment` object, which is required by the
`sprockets-helpers` gem to resolve asset paths.

The test that would cause errors is also in this commit diff. I've
updated it to be a bit more readable.

I also took this opportunity to review and clean up the `@allow_editing`
assignment, which seemed a bit obfuscated.

* Update migration script to test run warnings

When running the entire test suite, many warnings would be output:

/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: already initialized constant HYPHENATE
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: previous definition of HYPHENATE was here
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: already initialized constant PAGE_FILE_DIR
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: previous definition of PAGE_FILE_DIR was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: already initialized constant REPO
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: previous definition of REPO was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: already initialized constant TREE
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: previous definition of TREE was here
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: already initialized constant PAGE_FILE_DIR
/Users/bw/Projects/gollum/test/test_migrate.rb:37: warning: previous definition of PAGE_FILE_DIR was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: already initialized constant REPO
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:91: warning: previous definition of REPO was here
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: already initialized constant TREE
/Users/bw/Projects/gollum/bin/gollum-migrate-tags:236: warning: previous definition of TREE was here

While it's unlikely that end users would ever see these warnings, as
they'd only be running the migration script once, they will always be
shown in our local test runs and CI run output.

So instead of using constants in our migration script, I change the
script to use class variables instead. This should not effect the
functionality of the migration script whatsoever.

* Use `File.exist?` instead of `File.exists?`

In Ruby 3.x, `File.exists?` is deprecated and outputs a warning.

* Improve "allow editing" tests

While making changes to the test suite, I ran into some issues with
these tests failing on occasion.

I added some setup and teardown to help with this.

But one thing I did notice is that the word "Upload" appears in the
response body whether uploading is enabled or not, so I made these
assertions more specific to the HTML rather than other places the word
"Upload" might appear (which is related to Critic Markup).

* Do not attempt to modify frozen strings

Using `#gsub!` here now results in an error. I am not entirely sure why
this hasn't happened before, but the error is straightforward:

     FrozenError: can't modify frozen String: "Author %{author} is from %{location}"
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:45:in `gsub!'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:45:in `fill_argument_content'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:37:in `block in autofill'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `each'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `map'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `autofill'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:35:in `block in autofill'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `each'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `map'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:33:in `autofill'
        /home/runner/work/gollum/gollum/lib/gollum/views/helpers/locale_helpers.rb:21:in `t'
        /home/runner/work/gollum/gollum/test/gollum/views/test_locale_helper.rb:95:in `block (4 levels) in <top (required)>'

`#gsub!` attempts to modify a string in-place, which does not work when
a string is frozen. It seems that in some Ruby environments, strings
from YAML files are frozen.

* Fix another order-dependent test failure

Very occasionally, this test fails due to `Precious::App` settings set
in previous tests. You can reproduce this failure using this seed:

    bundle exec rake TESTOPTS="--seed=42898"
This commit is contained in:
benjamin wil
2022-04-27 08:25:54 -07:00
committed by GitHub
parent 81c90e55a7
commit 95d35d38da
10 changed files with 264 additions and 185 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bin/gollum-migrate-tags
+40 -28
View File
@@ -11,8 +11,11 @@ migrate_options = {
:hyphenate => true
}
def setting(const)
Object.const_defined?(const.upcase) && Object.const_get(const.upcase)
def setting(variable_name)
class_variable_name = :"@@#{variable_name.to_s}"
Object.class_variable_defined?(class_variable_name) &&
Object.class_variable_get(class_variable_name)
end
opts = OptionParser.new do |opts|
@@ -25,7 +28,7 @@ It finds and repairs Gollum link tags that no longer work under 5.x for three re
* 5.x wiki internal links are no longer 'global'.
* NB: you can use the --lenient-tag-lookup option in gollum >= 5.x to enable 4.x-backwards compatible tags.
See https://github.com/gollum/gollum/wiki/5.0-release-notes#filename-handling for more information.
Usage of this script comes without any warranty.
@@ -38,7 +41,7 @@ You can use the --page-file-dir and --config options as you would normally with
Requires a non-bare repository. Recommended usage:
1. Clone your wiki's repository to create a backup.
2. Run this script on your cloned repo.
2. Run this script on your cloned repo.
3. If all looks sane, run the script with the --write option. This will overwrite files in your working directory, but not commit the changes, so you have time to review them.
4. Do a 'git diff' to inspect the changes.
5. Commit the changes if all looks sane, and push/pull them back into your original repo.
@@ -52,23 +55,23 @@ EOF
opts.on('--page-file-dir [PATH]', 'Specify the subdirectory for all pages. Default: repository root.') do |path|
wiki_options[:page_file_dir] = path
end
opts.on('--prefer-relative-links', 'When specified, will try to replace broken links with relative links (\'[[Foo/Bar]]\' instead of \'[[/Subdir/Foo/Bar]]\') where possible.') do
migrate_options[:prefer_relative] = true
end
opts.on('--hyphenate', 'Default. Repair links that use spaces instead of hyphens: [[Bilbo Baggins]] -> [[Bilbo-Baggins]]') do
migrate_options[:hyphenate] = true
end
opts.on('--no-hyphenate', 'Turn off the --hyphenate option.') do
migrate_options[:hyphenate] = false
end
opts.on('--run-silent', 'Don\'t output anything.') do
migrate_options[:run_silent] = true
end
opts.on('--write', 'No dry run: actually perform the substitutions.') do
migrate_options[:no_dry_run] = true
end
@@ -78,8 +81,11 @@ end
begin
opts.parse!
migrate_options.each do |setting, value|
const = setting.to_s.upcase
Object.const_set(const, value) unless Object.const_defined?(const)
variable_name = :"@@#{setting.to_s}"
unless Object.class_variable_defined?(variable_name)
Object.class_variable_set(variable_name, value)
end
end
wiki_options[:page_file_dir] = setting(:page_file_dir) ? setting(:page_file_dir) : wiki_options[:page_file_dir] # Allow settings :page_file_dir through PAGE_FILE_DIR constant.
rescue OptionParser::InvalidOption
@@ -88,7 +94,7 @@ rescue OptionParser::InvalidOption
exit
end
REPO = ARGV[0] || Dir.pwd
wiki_directory = ARGV[0] || Dir.pwd
require 'gollum-lib'
@@ -98,7 +104,7 @@ if cfg = options[:config]
cfg = File.join(Dir.getwd, cfg) unless cfg.slice(0) == File::SEPARATOR
require cfg
end
class Gollum::Filter::CodeMigrator < Gollum::Filter::Code
def extract(data)
case @markup.format
@@ -126,7 +132,7 @@ class Gollum::Filter::CodeMigrator < Gollum::Filter::Code
next '' if m_end.length < m_start.length
lang = m_lang ? m_lang.strip.split.first : nil
cache_codeblock($~.to_s)
end
end
end
data.gsub!(/^([ ]{0,3})``` ?([^\r\n]+)?\r?\n(.+?)\r?\n[ ]{0,3}```[ \t]*\r?$/m) do
@@ -134,7 +140,7 @@ class Gollum::Filter::CodeMigrator < Gollum::Filter::Code
end
data
end
def process(data)
return data if data.nil? || data.size.zero? || @map.size.zero?
@map.each do |id, block| ## Just put the code blocks back in verbatim
@@ -142,23 +148,23 @@ class Gollum::Filter::CodeMigrator < Gollum::Filter::Code
end
data
end
def cache_codeblock(block)
id = "#{open_pattern}#{Digest::SHA1.hexdigest(block)}#{close_pattern}"
@map[id] = block
id
end
end
class ::Gollum::Filter::TagMigrator < Gollum::Filter::Tags
def process_tag(tag)
link_part, extra = parse_tag_parts(tag)
orig_tag = %{[[#{tag}]]}
return orig_tag if link_part.nil?
img_args = extra ? [extra, link_part] : [link_part]
mime = MIME::Types.type_for(::File.extname(img_args.first.to_s)).first
# For any kind of tag other than an internal link: just return the tag.
if tag =~ /^_TOC_/ || link_part =~ /^_$/ || link_part =~ /^#{INCLUDE_TAG}/ || (mime && mime.content_type =~ /^image/) || process_external_link_tag(link_part, extra) || process_file_link_tag(link_part, extra)
return orig_tag
@@ -168,7 +174,7 @@ class ::Gollum::Filter::TagMigrator < Gollum::Filter::Tags
link = link_part
page = find_page_or_file_from_path(link)
anchor = nil
if page.nil? # No match yet, now try finding the page with anchor removed
if pos = link.rindex('#')
anchor = link[pos..-1]
@@ -203,12 +209,12 @@ class ::Gollum::Filter::TagMigrator < Gollum::Filter::Tags
pick = possibles.first
return tag_for_pick(pick, orig_tag, extra, anchor, @markup.page.path)
end
end
end
private
def tag_for_pick(pick, orig_tag, extra, anchor, linking_page_path)
pick = if setting(:prefer_relative)
overlapping_path = Pathname.new(linking_page_path).dirname.to_s
@@ -220,7 +226,7 @@ class ::Gollum::Filter::TagMigrator < Gollum::Filter::Tags
end
new_tag = extra.nil? ? %{[[#{pick}#{anchor}]]} : %{[[#{extra}|#{pick}#{anchor}]]}
log(:info, "#{@markup.page.path}: Changing #{orig_tag} -> #{new_tag}")
new_tag
new_tag
end
end
@@ -232,8 +238,12 @@ end
filter_chain = [:PlainTextMigrator, :CodeMigrator, :TagMigrator]
wiki = ::Gollum::Wiki.new(REPO, wiki_options.merge({:filter_chain => filter_chain}))
TREE = wiki.tree_list(wiki.ref, true, true).map {|file| ::File.join('/', file.path)}
wiki = ::Gollum::Wiki.new(wiki_directory, wiki_options.merge({:filter_chain => filter_chain}))
Object.class_variable_set(
:"@@wiki_tree",
wiki.tree_list(wiki.ref, true, true).map {|file| ::File.join('/', file.path)}
)
def find_linked(link)
link.gsub!(' ', '-') if setting(:hyphenate) # Match paths containing dashes instead of spaces
@@ -243,7 +253,9 @@ def find_linked(link)
test_path = ::File.extname(link).empty? ? /#{link}\..+/ : link
# Select pages from the wiki whose path =~ 'Foo/Bar/Samwi.*'
# Match case-insenstively to mimic 4.x behavior!
TREE.select {|path| path =~ /^\/(.*\/)?#{test_path}/i}
Object.class_variable_get(:"@@wiki_tree").select { |path|
path =~ /^\/(.*\/)?#{test_path}/i
}
end
def log(kind, msg = nil)
@@ -268,4 +280,4 @@ wiki.pages.each do |page|
f.close
end
log(:none, '====')
end
end