In the editor, the help text for "Emoji" included a dead link. This
changes the link to a non-dead link.
After changing the JS, assets needed to be recompiled.
Resolves#1775.
There was an order-dependent test failure that plagued a couple of my
recent pull requests:
Failure:
Default_keybindings#test_keybinding_unset
[/home/runner/work/gollum/gollum/test/test_app.rb:1098]
Minitest::Assertion: Expected: true
Actual: false
This commit ensures that `default_keybinding` is set to `nil` before
each test, and that the first line of each test changes the
configuration to the setting under test explicitly.
I also refactored the teardown to `nil` out all instance values so
they'll all certainly be reset when the next set of tests is run.
On my system, Chrome is not in the location that the
`Selenium::WebDriver` expects it to be by default. This configuration
setting lets a developer set the path to the Chrome binary explicitly if
necessary.
Any Capybara-related test setup has been moved to `capybara_helper.rb`.
Now we just must require this file in order to run tests with Capybara.
I also moved the `#console_log` helper to this file to showcase how
other global helper methods could be defined in this file.
I saw a random (??) failure on the test on line 1098. This trivial chang
e just ensures that the configuration under test (the default
keybinding) is explicitly being set to `nil` after test runs.
In issue #1815, it was reported that Gollum RSS feeds attempt to put
entire commit messages into a feed item's `<title>` element.
Given that commit messages can be many paragraphs long, this is not an
acceptable way to render a feed item title.
If a commit has many lines or paragraphs, we now put those inside the
`<description>` element as of this commit.
While I was editing this view, I decided it would be a good time to
increase test coverage, as this view renderer was not under test at
all. I also fixed a typo ("Commited" should say "Committed"). There are
implicit dependencies on other gems that provide the
`Gollum::Git::Commit`, `Gollum::Git::Actor`, and `Rugged::Commit`
classes here, so mocking out simple versions of their interfaces seemed
like the path of least resistance to setting up controllable tests.
I also removed some now-duplicated assertions from `test/test_app.rb`.
* Internationalize `Views::Compare` templates
* Internationalize `Views::Error` templates
* Internationalize `Views::History` templates
* Internationalize `Views::LatestChanges` templates
* Internationalize `Views::Layout` templates
* Internationalize `Views::Overview` templates
* Internationalize `Views::Search` templates
* Reset I18n load path after I18n helper tests
* Create locale helper for global translations
There are some translation strings we should use across multiple views.
Instead of duplicating the translations, we can use a different locale
helper method, `#tt`, to get a hash of all available translations.
Then, in a partial view like `pagination.mustache`, we can render
translations regardless of what the current view class is.
This commit adds the necessary helper, tests, and uses the new method to
render translations on the `pagination.mustache` template, which is used
by many other view classes (latest changes, history, and search).
* Start using `yarn` for vendor assets
Until this commit, Gollum has included JavaScript assets by committing
the source code to the `lib/gollum/public/gollum/javascript/` directory
and including them for compilation in the Sprockets asset manifest file
at `lib/gollum/public/gollum/javascript/app.js`. This has been a
reasonable way to deal with third-party JavaScript, but there are a few
downsides:
- It's a burden to find out if JavaScript dependencies have been
updated and require updating in Gollum.
- It doesn't give us good visibility into the JavaScript dependencies
required by our dependencies.
- It forces us to commit external code to our repository, which can
make our developer tools more difficult to configure or use. For
example: when I search for key words in the repository using
Ripgrep, I often get "garbage" results from minified JavaScript.
Managing JavaScript dependencies via a JS package manager can resolve
all of these issues.
This commit allows us to manage JavaScript dependencies using the Yarn
package manager for Node JS modules[1]. I chose Yarn over NPM for one
reason: Yarn is the JavaScript package manager that Rails uses by
default. So many Ruby developers will already be familiar with Yarn.
To demonstrate how this can change how we manage JS assets in Gollum,
I've configured Yarn and started to manage the `mousetrap` dependency
with it. I chose `mousetrap` to start because it's a smaller, mostly
uncomplicated dependency. I was easily able to manually test that
`mousetrap` is still working after re-compiling the assets.
Hopefully this gives anyone reading enough context to jump in and start
moving our third-party JS assets out of the codebase.
[1]: https://yarnpkg.com/
* Recompile assets
* Add dev environment setup info to CONTRIBUTING
Now that we require additional tooling to manage JavaScript
dependencies, it seemed reasonable to add more documentation around
setting up one's development environment.
* Don't compile assets without `yarn`-managed ones
If a developer compiled assets without first running `yarn install`, we
would get incomplete collection of up-to-date assets.
Let's ensure that the developer has all the required assets by enforcing
this in the precompile task they should be using to compile production
assets to begin with.
We've had some reports (#1342, #1809) of `rdoc` not being installed
in some environments, causing Gollum to error in unpleasant ways.
This isn't an issue that's unique to Gollum, and I don't think `rdoc`
should actually be a hard dependency of this project. But since we can
side-step the issue by requiring `rdoc`, something that most users who
install Gollum via RubyGems would already have installed, I don't think
this is a horrible solution.
* Ensure example git repos are valid
It was reported in #1817 that the `lotr_migration.git` repository we use
in `test/test_migrate.rb` is not a valid git repository on when clone,
causing the test suite to fail when run locally.
The reason is because there is no `.git/refs` directory, meaning it's
not really a valid git repository at all. I noticed that the `empty.git`
example repository has the same problem.
This commit simply ensures that the directory structure of these example
repositories are persisted in git.
* Run `test_migrate` tests on CI
Now that we've resolved the issue with the invalid git repository in the
parent commit, we can run all of the tests in our CI environment.
* 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"
benjamin wil
Dawa Ometto
Josh Soref
benjamin wil
kyle tibbetts
Dawa Ometto
Paul Westcott
Andreas Wachter
Tevin
mishina