Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Simplify default rake test loader #357

Merged
merged 1 commit into from
Oct 22, 2020
Merged

Simplify default rake test loader #357

merged 1 commit into from
Oct 22, 2020

Conversation

deivid-rodriguez
Copy link
Contributor

Unless I'm missing something, we can require rake_test_loader directly. It's also safer, because there's no chance of requiring the file of a different copy of rake, and faster.

Unless I'm missing something, we can require `rake_test_loader`
directly. It's also safer, because there's no chance of requiring the
file of a different copy of `rake`, and faster.
@deivid-rodriguez
Copy link
Contributor Author

Does any maintainer have any opinion about this?

@hsbt
Copy link
Member

hsbt commented Oct 22, 2020

✋ I'm not sure why the current implementation required. But your changes seems fine.

@hsbt hsbt merged commit 5947d20 into ruby:master Oct 22, 2020
@deivid-rodriguez deivid-rodriguez deleted the simplify_rake_test_loader branch October 22, 2020 11:31
ysakasin added a commit to ysakasin/rake that referenced this pull request Dec 20, 2020
Due to ruby#357, execution order on Rake 13.0.2 changes from Rake 13.0.1.

Example:
```
Rake::TestTask do |t|
  t.test_files = [
    "test/a.rb",
    "test/b.rb",
  ]
end
```

On 13.0.2, Rake executes test/b.rb before test/a.rb because test/a.rb are loaded before rake_test_loader.rb.
rake_test_loader.rb executes the Ruby code in ARGV using Kernel.#require, but does not execute test/a.rb which is already loaded.

In addition, Rake 13.0.1 allows specifying test_files without .rb but 13.0.2 doesn't allows.
This commit also fixes this problem.
```
Rake::TestTask do |t|
  t.test_files = [
    "test/setup",
    "test/a.rb",
  ]
end
```
This was referenced Mar 15, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants