This makes test file patterns configurable via two environment variables:
`DEFAULT_TEST`, to configure files to test, and `DEFAULT_TEST_EXCLUDE`,
to configure files to exclude from testing.
These values were hardcoded before, which made it difficult to add
new categories of tests that should not be executed by default (e.g:
smoke tests).
It uses environment variables instead of regular Rails config options
because Rails environment is not available when the Runner builds the
list of files to test (unless using Spring). A nicer solution would be
making sure that the Rails environment is always loaded when the runner
starts. This is a first simple step to make these paths configurable for
now
This way at least you could override defaults in `config/boot.rb`:
```ruby
ENV["DEFAULT_TEST_EXCLUDE"] = "test/{dummy,smoke,system}/**/*_test.rb
```
Co-authored-by: Jeremy Daer <jeremydaer@gmail.com>
In the past, we sometimes hit missing `Symbol#start_with?` and
`Symbol#end_with?`.
63256bc5d7a8e812964d
So I proposed `Symbol#start_with?` and `Symbol#end_with?` to allow duck
typing that methods for String and Symbol, then now it is available in
Ruby 2.7.
https://bugs.ruby-lang.org/issues/16348
Using `String#starts_with?` and `String#ends_with?` could not be gained
that conveniency, so it is preferable to not use these in the future.
Previously, a test runner argument had to contain a forward slash to be
recognized as a path. Now, backslashes will also be considered,
enabling the use of Windows-style paths.
Fixes#38243.
Mandrill's Inbound API checks to see if a URL exists before it creates
the webhook. It sends a HEAD request, to which we now return a 200 OK
response to indicate that the route exists.
Now we can generate inbound API calls with ease on Mandrill, without
having to shuffle around tokens in production.
Fixes#37609.
`url_for` will now use "https://" as the default protocol when
`Rails.application.config.force_ssl` is set to true.
Action Mailer already behaves this way, effectively. This commit
extends that behavior application-wide.
Closes#23543.
These were removed in 74201c3885ae2e33bfff046d503324fd1d7a320f when the
template for the 6.0 new framework defaults initializer was deleted.
While we no longer generate the file, upgrading applications will still
have it, so it's still important to check that these options can be set.
I introduced this pattern of referencing a constant to trigger lazy load
hooks in 458a5502a17ccf58d5708a3b030ac9917a0a8476, and it arrived at its
current form via c98a641ff402d3ca5b754f4621a0764f33eab155 and
c24be369322b9e0211fcef30003375de195ef660.
I now realise autoloading doesn't need to be involved at all; we can
require the files that trigger the lazy load hooks directly.
* Add a way to deliver inbound emails by source
Great for testing when you get an eml file and can just paste it in.
* Test updates
* Fix tests
* Fix spacing
* require, require_relative, load by double quotes
We're getting rid of all single quote usage, unless it serves a specific purpose, as per the general style guide.
The `rails test` command scans its arguments for test paths to load
before handing off option parsing to Minitest. To avoid incorrectly
interpreting a `-n /regex/` pattern as an absolute path to a directory,
it skips arguments that end with a slash. However a relative path ending
in a slash is not ambiguous, so we can safely treat those as test paths.
This is especially useful in bash, where tab completing a directory
leaves a trailing slash in place.
With a multiple database application `db:rollback` becomes problematic.
We can't rollback just the primary, that doesn't match the behavior in
the other tasks. We can't rollback a migration for every database, that
is unexpected.
To solve this I handled `db:rollback` the same way I handled `:up` and
`:down`. If `db:rollback` is called for a multi-db application then it
will raise an error recommending you use `db:rollback:[NAME]` instead.
Calling `db:rollback:primary` or `db:rollback:animals` will rollback
the migration for the number of steps specified.
Closes: #38513
Follow-up to: #34078
This method was jumping through extra hoops to find the name of the
class the connection is stored on when we can get it from the connection
itself. Since we already have the connection we don't need to loop through the
pools.
In addition, this was using the wrong class name. The class name for the
schema migration should come from the connection owner class, not from
the `db_config.name`. In this case, `db_config.name` is the name of the
configuration in the database.yml. Rails uses the class name to lookup
connections, not the db config name, so we should be consistent here.
While working on this I noticed that we were generating an extra schema
migration class for `ActiveRecord::Base`. Since `ActiveRecord::Base` can
and should use the default and we don't want to create a new one for
single db applications, we should skip creating this if the spec name is
`ActiveRecord::Base`. I added an additional test that ensures the class
generation is correct.
If I had had these tests previously I would have not created PR #38658
and then promptly realize I needed to revert it.
We need to load and parse the configurations twice. Once before the
environment is loaded to create the named tasks and once after the
environment is loaded to have the real configurations. The configs
loaded before the env have the ERB stripped out and aren't valid
configs.
I have so. many. regrets. about using `spec_name` for database
configurations and now I'm finally putting this mistake to an end.
Back when I started multi-db work I assumed that eventually
`connection_specification_name` (sometimes called `spec_name`) and
`spec_name` for configurations would one day be the same thing. After
2 years I no longer believe they will ever be the same thing.
This PR deprecates `spec_name` on database configurations in favor of
`name`. It's the same behavior, just a better name, or at least a
less confusing name.
`connection_specification_name` refers to the parent class name (ie
ActiveRecord::Base, AnimalsBase, etc) that holds the connection for it's
models. In some places like ConnectionHandler it shortens this to
`spec_name`, hence the major confusion.
Recently I've been working with some new folks on database stuff and
connection management and realize how confusing it was to explain that
`db_config.spec_name` was not `spec_name` and
`connection_specification_name`. Worse than that one is a symbole while
the other is a class name. This was made even more complicated by the
fact that `ActiveRecord::Base` used `primary` as the
`connection_specification_name` until #38190.
After spending 2 years with connection management I don't believe that
we can ever use the symbols from the database configs as a way to
connect the database without the class name being _somewhere_ because
a db_config does not know who it's owner class is until it's been
connected and a model has no idea what db_config belongs to it until
it's connected. The model is the only way to tie a primary/writer config
to a replica/reader config. This could change in the future but I don't
see value in adding a class name to the db_configs before connection or
telling a model what config belongs to it before connection. That would
probably break a lot of application assumptions. If we do ever end up in
that world, we can use name, because tbh `spec_name` and
`connection_specification_name` were always confusing to me.
Follow-up to #38463.
By isolating ARGV, we guard against commands inadvertently depending on
prior ARGV contents. Any such command will now behave consistently when
run via `Rails::Command.invoke`, whether coming from the Rails CLI or
from library code. Likewise, any ARGV mutations done by a command will
not affect code that executes after `Rails::Command.invoke`.
The schema cache tests test the following scenarios:
1) The default case works (single db, primary spec name (dev is default
to primary in 2-tier config), standard default schema cache filename)
2) Primary always wins over other entries
3) A custom schema cache filename works when set in the configuration
4) A custom schema cache filename works when set in the ENV
Cases that don't work:
1) A non-primary database entry picks up a namespaced schema cache file
This can't work currently because there's no way of knowing which cache
we actually want. In this railtie we can only load ActiveRecord::Base's
schema cache. If we grab the first config we risk loading a cache for
another connection because order is not guaranteed.
2) Multi-db schema caches
The reasons are similar to above. In addition we can't loop through the
configs, establish a connection, and load the cache because we don't
know what parent class to establish a connection to. In that case AR
Base will always get the cache and it would cause the last one to win
and therefore be loaded on the wrong connection.
The real fix for these issues is to get rid of the railtie entirely, but
for now we needed to set this back to what the behavior was before
recent changes but with the ability to pass a custom key.
Co-authored-by: Katrina Owen <kytrinyx@github.com>
The initializer that loads the default schema cache on the default
connection doesn't account for the case where an app overrides the
default filename either via ENV["SCHEMA_PATH"], or via the
:schema_cache_path defined in the db config.
Note that as discussed in #34449 this initializer doesn't work for
applications using multiple databases, and this change doesn't fix that.
Database configurations are now objects almost everywhere, so we don't
need to fake access to a hash with `#default_hash` or it's alias `#[]`.
Applications should `configs_for` and pass `env_name` and `spec_name` to
get the database config object. If you're looking for the default for
the test environment you can pass `configs_for(env_name: "test", spec_name:
"primary")`. Change test to developement to get the dev config, etc.
`#default_hash` and `#[]` will be removed in 6.2.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
The main interface to eager loading is config.eager_load. The logic that
implies happens during the boot process.
With the introduction of Zeitwerk, application code is loaded in the
finisher as everything else, but in previous versions of Rails users
could eager load the application code regardless of config.eager_load.
Use cases:
* Some gems like indexers need to have everything in memory and would
be a bad user experience to ask users to conditionally set the eager
load flag.
* Some tests may need to have everything in memory and would be a bad
experience to have the flag enabled globally in the test environment.
I personally feel that the contract between this method and the entire
eager loading process is ill-defined. I believe this method is
essentially internal. The purpose of this patch is simply to restore this
functionality emulating what it did before because rethinking the design
of this interface may need time.
Fixes https://github.com/rails/rails/issues/28827.
The steps to reproduce are as follows:
git clone git@github.com:bbuchalter/rails-issue-28827.git
cd rails-issue-28827
bundle install
bin/rails db:create
Observe that we create two databases when invoking db:create: development and test. Now observe what happens when we invoke our drop command while using DATABASE_URL.
DATABASE_URL=sqlite3://$(pwd)/db/database_url.sqlite3 bin/rails db:create
As expected, the development environment now uses the DATABASE_URL. What is unexpected is that the test environment does not.
It's unclear what the expected behavior should be in this case, but the cause of it is this: 9f2c74eda0/activerecord/lib/active_record/tasks/database_tasks.rb (L494)
Because of each_local_configuration, there seems to be no way invoke these database rake on only the development environment to ensure DATABASE_URL is respected.
The smallest scope of change I can think to make would be to conditionalize this behavior so it does not get applied when DATABASE_URL is present.
- ### Problem
ActionPack requires "action_view/base" at boot time, this
causes a variety of issue that I described in detail in #38024.
There is no real reason to require av/base in the
ActionDispatch::Debugexceptions class.
### Solution
Like any other components (such as ActiveRecord, ActiveJob...),
ActionView::Base shouldn't be loaded at boot time.
Here are the two main changes needed for this:
1) Actionview has a special initializer that needs to run
before the app is fully booted (adding a executor needs to be done
before application is done booting)
63ec70e700/actionview/lib/action_view/railtie.rb (L81-L84)
That initializer used a lazy load hooks but we can't do that anymore
because Action::Base view won't be triggered during booting process.
When it will get triggered, (presumably on the first request),
it's too late to add an executor.
------------------------------------------------
2) Compare to other components, ActionView doesn't use `Base` for
configuration flag. A lot of flags ares instead set on modules
(FormHelper, FormTagHelper).
The problem is that those module depends on AV::Base to be
loaded, as otherwise configuration set by the user aren't applied.
(Since the lazy load hooks hasn't been triggered)
63ec70e700/actionview/lib/action_view/railtie.rb (L66-L69)
We shouldn't wait for AB::Base to be loaded in order to set these
configuration. However, we need to do it inside an
`after_initialize` block in order to let application
set it to the value they want.
Closes#28538
Co-authored-by: betesh <iybetesh@gmail.com>"
We missed these in rails/rails#38005 because deprecation warnings are
silently swallowed by these tests.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Enabling `SameSite` cookie protection is an addition to CSRF protection,
where cookies won't be sent by browsers in cross-site POST requests when set to `:lax`.
`:strict` disables cookies being sent in cross-site GET or POST requests.
Passing `:none` disables this protection and is the same as previous versions albeit a `; SameSite=None` is appended to the cookie.
See upgrade instructions in config/initializers/new_framework_defaults_6_1.rb.
More info [here](https://tools.ietf.org/html/draft-west-first-party-cookies-07)
_NB: Technically already possible as Rack supports SameSite protection, this is to ensure it's applied to all cookies_
- ### Problem
```ruby
MyJob < ApplicationJob
before_enqueue { throw(:abort) }
after_enqueue { # enters here }
end
```
I find AJ behaviour on after_enqueue and after_perform callbacks
weird as they get run even when the callback chain is halted.
It's counter intuitive to run the after_enqueue callbacks even
though the job wasn't event enqueued.
### Solution
In Rails 6.2, I propose to make the new behaviour the default
and stop running after callbacks when the chain is halted.
For application that wants this behaviour now or in 6.1
they can do so by adding the `config.active_job.skip_after_callbacks_if_terminated = true`
in their configuration file.
= This feature existed back in 2012 5e7d6bba79
but got reverted with the incentive that there was a better approach.
After discussions, we agreed that it's a useful feature for apps
that have a really large set of routes.
Co-authored-by: Yehuda Katz <wycats@gmail.com>
This reverts commit 28e44f472d1cd6853726f85eeb7623e5901c4d37.
A limitation of Listen is that it currently only supports watching directories.
Therefore, watching `config/routes.rb` will end up watching the entire `config` directory
if we use the evented file watcher. This causes problems especially if symlinks are present
in the `config` directory.
- The migrate task iterates and establish a connection over each db
resulting in the last one to be used by subsequent rake tasks.
We should reestablish a connection to the connection that was
established before the migrate tasks was run
- Fix#37578
Make has_many inversing support available through an opt-in config
variable. This behaviour is likely to break existing applications, but
it is correct behaviour.
Services can be configured in `config/storage.yml` with a new key
`public: true | false` to indicate whether a service holds public
blobs or private blobs. Public services will always return a
permanent URL.
Deprecates `Blob#service_url` in favor of `Blob#url`.
We no longer link all js by default, so we should do this test with a
css instead (we don't care about that specifics of the dir just that its
in the manifest and in this dir).
It fixes the problem in propagating return_only_media_type_on_content_type
and fixes the corresponding test being ineffective.
The mentioned test addes the following line:
...config.action_dispatch.return_only_media_type_on_content_type = true
to the config and checks if it takes effect. However, in this scenario,
the value is already true before this line.
Moreover, the users are supposed to flip this from true to false in real
situations.
This commit flips the config in the test, making it to fail as
expected. The next commit will fix the failure.
In order for return_only_media_type_on_content_type to appropriately
take effect on ActionDispatch::Response, we want to know when
ActionDispatch::Response is loaded.
As load hooks for ActionDispatch would be too broad, the appropriate
registry is for ActionDispatch::Response itself.
Looking into other examples, a hook name is a full class name in
snake case with `_base` suffix omitted, if any. Therefore, in this case,
:action_dispatch_response seems appropriate.
We are seeing some test failures for this test in #37291. It looks like
what's going on is that Puma has changed the output for this command
between 4.1 and 4.2
Previously:
```
...
* Environment: development
* Listening on tcp://localhost:3000
...
```
Now:
```
...
* Environment: development
* Listening on tcp://127.0.0.1:3000
* Listening on tcp://[::1]:3000
...
```
So to get around this, instead of checking the binding address, just
check for the presence of 'Listening' generally like we do on server
start.
Co-authored-by: eileencodes <eileencodes@gmail.com>
Convert all uses of `db_config.configuration_hash[*]` to use methods
defined on an implementation of `DatabaseConfigurations::DatabaseConfig`.
Since we want to get away from accessing properties directly on the
underlying configuration hash, we'll move here to accessing those values
via the implementations on `DatabaseConfig` (or more specifically,
`HashConfig`).
There are still codepaths that are passing around `configuration_hash`,
and follow-on PRs will address those with the goal of using
configuration objects everywhere up until the point we pass a resolved
hash over to the underlying client.
Co-authored-by: eileencodes <eileencodes@gmail.com>
Eventually we'd like to get rid of this class altogether but for now
this PR reduces the surface area by removing methods from the class and
moving classes out into their own files.
* `adapter_method` was moved into database configurations
* `initialize_dup` was removed because it was only used in tests
* Resolver is now it's own class under connection adapters
* ConnectionUrlResolver, only used by the configurations, is in a class
under DatabaseConfigurations
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Previously in some places we used symbol keys, and in some places we used
string keys. That made it pretty confusing to figure out in a particular
place what type of configuration object you were working with.
Now internally, all configuration hashes are keyed by symbols and
converted to such on the way in.
A few exceptions:
- `DatabaseConfigurations#to_h` still returns strings for backward compatibility
- Same for `legacy_hash`
- `default_hash` previously could return strings, but the associated
comment mentions it returns symbol-key `Hash` and now it always does
Because this is a change in behavior, a few method renames have happened:
- `DatabaseConfig#config` is now `DatabaseConfig#configuration_hash` and returns a symbol-key `Hash`
- `ConnectionSpecification#config` is now `ConnectionSpecification#underlying_configuration_hash` and returns the `Hash` of the underlying `DatabaseConfig`
- `DatabaseConfig#config` was added back, returns `String`-keys for backward compatibility, and is deprecated in favor of the new `configuration_hash`
Co-authored-by: eileencodes <eileencodes@gmail.com>
Currently, autoload paths pass to the watcher as directories. If using evented
watcher, this possibly pass as it is to `Listen`.
But autoload paths include files and `Listen` raise an error when was passed
file. So, it is necessary to classify files and directories correctly.
Fixes#37011.
Previously, we used the migration status to determine whether the test
database(s) needed to be reloaded from the schema. This worked in most
cases, but if a schema.rb was modified outside of migrations or if a
migration was rolled back, it would require a manual db:test:prepare.
This commit updates load_schema to record the SHA1 of the loaded schema
file inside of the ar_internal_metadata table. We can then use this SHA
to determine whether we should reload the schema.
This ensures that the test DB stays exactly in sync with the schema
file, including rollbacks which fixes a test marked TODO.
The `rake db:seed` command was broken for the primary environment if the
application is using multiple databases. We never implemented `rake
db:seed` for other databases (coming soon), but that shouldn't break the
default case.
The reason this was broken was because `abort_if_pending_migrations`
would loop through the configs for all databases and check for
migrations but would leave the last established connection. So `db:seed`
was looking in the wrong database for the table to seed.
This PR doesn't fix the fact that `db:seed` doesn't work for multiple
databases but does fix the default case.
Fixes#36817
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Applications are not supposed to use require_dependency in their own
code if running in zeitwerk mode, and require_dependency was initially
aliased to require with that use case in mind.
However, there are situations in which you cannot control the mode and
need to be compatible with both. There, you might need require_dependency
in case you are being executed in classic mode. Think about engines that
want to support both modes in their parent applications, for example.
Furthermore, Rails itself loads helpers using require_dependency.
Therefore, we need better compatibility.
* read config/webpacker.yml to determine which path to exclude for zeitwerk:check
* fix test errors
* more changes to fix test errors
* refactor webpacker_path
[Andrew Kress + Rafael Mendonça França]
- I made a change in https://github.com/rails/rails/pull/36691 to
delegate route helper to a proxy class.
This didn't take into account that the `url_options` we redefine
in SystemTest would be ignored.
This PR fixes that by definin the url_options inside the proxy
A HTTP feature policy is Yet Another HTTP header for instructing the
browser about which features the application intends to make use of and
to lock down access to others. This is a new security mechanism that
ensures that should an application become compromised or a third party
attempts an unexpected action, the browser will override it and maintain
the intended UX.
WICG specification: https://wicg.github.io/feature-policy/
The end result is a HTTP header that looks like the following:
```
Feature-Policy: geolocation 'none'; autoplay https://example.com
```
This will prevent the browser from using geolocation and only allow
autoplay on `https://example.com`. Full feature list can be found over
in the WICG repository[1].
As of today Chrome and Safari have public support[2] for this
functionality with Firefox working on support[3] and Edge still pending
acceptance of the suggestion[4].
#### Examples
Using an initializer
```rb
# config/initializers/feature_policy.rb
Rails.application.config.feature_policy do |f|
f.geolocation :none
f.camera :none
f.payment "https://secure.example.com"
f.fullscreen :self
end
```
In a controller
```rb
class SampleController < ApplicationController
def index
feature_policy do |f|
f.geolocation "https://example.com"
end
end
end
```
Some of you might realise that the HTTP feature policy looks pretty
close to that of a Content Security Policy; and you're right. So much so
that I used the Content Security Policy DSL from #31162 as the starting
point for this change.
This change *doesn't* introduce support for defining a feature policy on
an iframe and this has been intentionally done to split the HTTP header
and the HTML element (`iframe`) support. If this is successful, I'll
look to add that on it's own.
Full documentation on HTTP feature policies can be found at
https://wicg.github.io/feature-policy/. Google have also published[5] a
great in-depth write up of this functionality.
[1]: https://github.com/WICG/feature-policy/blob/master/features.md
[2]: https://www.chromestatus.com/feature/5694225681219584
[3]: https://bugzilla.mozilla.org/show_bug.cgi?id=1390801
[4]: https://wpdev.uservoice.com/forums/257854-microsoft-edge-developer/suggestions/33507907-support-feature-policy
[5]: https://developers.google.com/web/updates/2018/06/feature-policy
Until Rails 5.2, generators can run same name multi times without destroying.
But Rails 6.0(with Zeitwerk) can't this. In Rails 6.0, an error occurs
due to class name collision check.
The check uses `const_defined?`, which assumes that the autoload object
is also defined.
https://ruby-doc.org/core-2.6.3/Module.html#method-i-const_defined-3F
It did not work until Rails 5.2, but Zeitwerk seems to be able to
correctly check this against the application's code.
However, this is a little inconvenient if want to run the generator
again like mistake an attribute name(need to run `destoy` before).
In order to solve this, this PR adds an option to skip the collision check.
With this option, you can overwrite files just as did until Rails 5.2.
For multiple databases we attempt to generate the tasks by reading the
database.yml before the Rails application is booted. This means that we
need to strip out ERB since it could be reading Rails configs.
In some cases like https://github.com/rails/rails/issues/36540 the ERB
is too complex and we can't overwrite with the DummyCompilier we used in
https://github.com/rails/rails/pull/35497. For the complex causes we
simply issue a warning that says we couldn't infer the database tasks
from the database.yml.
While working on this I decided to update the code to only load the
database.yml once initially so that we avoid having to issue the same
warning multiple times. Note that this had no performance impact in my
testing and is merely for not having to save the error off somewhere.
Also this feels cleaner.
Note that this will not break running tasks that exist, it will just
mean that tasks for multi-db like `db:create:other_db` will not be
generated. If the database.yml is actually unreadable it will blow up
during normal rake task calls.
Fixes#36540
I changed to set CSP nonce to `style-src` directive in #32932.
But this causes an issue when `unsafe-inline` is specified to `style-src`
(If a nonce is present, a nonce takes precedence over `unsafe-inline`).
So, I fixed to nonce directives configurable. By configure this, users
can make CSP as before.
Fixes#35137.
I changed return value of `ActionDispatch::Response#content_type` in #36034.
But this change seems to an obstacle to upgrading. https://github.com/rails/rails/pull/36034#issuecomment-498795893
Therefore, I restored the behavior of `ActionDispatch::Response#content_type`
to 5.2 and deprecated old behavior. Also, made it possible to control the
behavior with the config.
This PR moves the `schema_migration` to `migration_context` so that we
can access the `schema_migration` per connection.
This does not change behavior of the SchemaMigration if you are using
one database. This also does not change behavior of any public APIs.
`Migrator` is private as is `MigrationContext` so we can change these as
needed.
We now need to pass a `schema_migration` to `Migrator` so that we can
run migrations on the right connection outside the context of a rake
task.
The bugs this fixes were discovered while debugging the issues around
the SchemaCache on initialization with multiple database. It was clear
that `get_all_versions` wouldn't work without these changes outside the
context of a rake task (because in the rake task we establish a
connection and change AR::Base.connection to the db we're running on).
Because the `SchemaCache` relies on the `SchemaMigration` information we
need to make sure we store it per-connection rather than on
ActiveRecord::Base.
[Eileen M. Uchitelle & Aaron Patterson]
When someone has a multi-db application their `ApplicationRecord` will
look like:
```ruby
class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
connects_to database: { writing: :primary, reading: :replica }
end
```
This will cause us to open 2 connections to ActiveRecord::Base's
database when we actually only want 1. This is because Rails sees
`ApplicationRecord` and thinks it's a new connection, not the existing
`ActiveRecord::Base` connection because the
`connection_specification_name` is different.
This PR changes `ApplicationRecord` classes to consider themselves the
same as the "primary" connection.
Fixes#36382
Previously, if a test worker exited early, the in-flight test it was
supposed to run wasn't reported as a failure.
If all workers exited immediately, this would be reported as ex.
Finished in 1.708349s, 39.2192 runs/s, 79.0237 assertions/s.
67 runs, 135 assertions, 0 failures, 0 errors, 2 skips
This commit validates that all workers finish running tests by ensuring
that the queue is empty after they exit. This works because we signal
the workers to exit by pushing nil onto the queue, so that there should
be a number of items left in the queue matching potentially missed
tests.
*sigh* this seems like the never ending bug. I don't love or even like
this fix but it does _work_.
Rafael suggested using `dummy_key: dummy_value` but unfortunately
that doesn't work. So we're left with checking whethere there might be
ternary type things in the content and then assuming that we want to
replace the line with a key value pair.
Technically fixes https://github.com/rails/rails/issues/36088
This commit more or less undoes 9b5401f, restores autoloaded? not to
touch the descendants tracker, and autoloaded_constants because it is
documented in the guide.
Previously we were calling the `take_failed_screenshot` method in an
`after_teardown` hook. However, this means that other teardown hooks
have to be executed before we take the screenshot. Since there can be
dynamic updates to the page after the assertion fails and before we
take a screenshot, it seems desirable to minimize that gap as much as
possible. Taking the screenshot in a `before_teardown` rather than an
`after_teardown` helps with that, and has a side benefit of allowing
us to remove the nested `ensure` commented on here:
https://github.com/rails/rails/pull/34411#discussion_r232819478
This change adds the ability to run up/down for a database in a multi-db
environment.
If you have an app with a primary and animals database the following
tasks will be generated:
```
VERSION=123 rake db:migrate:up:primary
VERSION=123 rake db:migrate:up:primary
VERSION=123 rake db:migrate:down:primary
VERSION=123 rake db:migrate:up:animals
```
I didn't generate descriptions with them since we don't generate a
description for a single database application.
In addition to this change I've made it so if your application has
multiple databases Rails will raise if you try to run `up` or `down`
without a namespace. This is because we don't know which DB you want to
run `up` or `down` against unless the app tells us, so it's safer to
just block it and recommend using namespaced versions of up/down
respectively.
The output for the raise looks like:
```
You're using a multiple database application. To use `db:migrate:down`
you must run the namespaced task with a VERSION. Available tasks are
db:migrate:down:primary and db:migrate:down:animals.
```
Currently ActionController::API doesn't include Caching module, so it
can't perform caching. And even if users include it later manually, it
won't inherit application's default cache store for action_controllers.
So the only way to solve this issue is to include Caching module in
ActionController::API, too.
This closes#35602
`bin/setup` and `bin/update` are currently almost the same file. The
only thing that keeps them apart is that one is running `bin/rails
db:setup` and the other `bin/rails db:migrate`.
I'm suggesting here that they should be a unique script, which needs to
be idempotent.
- New to a project, need to get started? `bin/setup`
- Need to install new dependencies that were added recently? `bin/setup`.
Before deprecating `bin/update`, I'm suggesting we just have it call
`bin/setup`.
The original message from Zeitwerk is "can't reload, please call
loader.enable_reloading before setup (Zeitwerk::Error)", which is not
very informative for Rails programmers.
Rails should err with a message worded in terms of its interface.
During initialization, the eager load paths of engines are unshifted
into AS::Dependencies.autoload_paths. After that, the collection is
frozen. (See the initializers in railties/lib/rails/engine.rb.)
Hence, there is no eager load path that is not an autoload path too, and
so the array difference in the deleted code is always an empty array.
Just do nothing.
A long-running `rails console --sandbox` could cause a database server
to become out-of-memory as it's holding on to changes that happen on the
database.
Given that it's common for Ruby on Rails application with huge
traffic to have separate write database and read database, we should
allow the developers to disable this sandbox option to prevent someone
from accidentally causing the Denial-of-Service on their server.
Since 3777701f1380f3814bd5313b225586dec64d4104, the environment's name is
automatically expanded in console and dbconsole commands.
In order to match the behavior between the commands, fixes it to have the
same behavior of all the commands.
This behavior is defined in `EnvironmentArgument`. Since
`EnvironmentArgument` also defines the environment option, it is reused.
However, since desc was not content that can be used in all comments,
fixed desc to be defined for each command.
The tmp directory is added to version control in the newly created
application. This was added in Rails 5.0.0(f06ce4c12a).
However, applications created before that are not guaranteed to have the
tmp directory. If the tmp directory does not exist, writing to the key file
raise error.
This is a bit incompatible. So I fixed that create the directory before
writing a key.
This was missed in the security fix for local dev. CI doesn't have a tmp
directory in the apps built for testing so these end up failing. This
adds the secret_key_base so we don't need to generate one.
This adds a few additional tests to the commits by eileencodes (https://github.com/rails/rails/pull/35497) and rafaelfranca (cfa22f1a4b). The additional tests cover several more ERB tag formatting cases such as multiline tags, conditional statements that result in duplicate keys, and multiple erb statements on a single line.
If the secret_key_base is nil in dev or test generate a key from random
bytes and store it in a tmp file. This prevents the app developers from
having to share / checkin the secret key for dev / test but also
maintains a key between app restarts in dev/test.
[CVE-2019-5420]
Co-Authored-By: eileencodes <eileencodes@gmail.com>
Co-Authored-By: John Hawthorn <john@hawthorn.email>
This change adds a new method that loads the YAML for the database
config without parsing the ERB. This may seem odd but bear with me:
When we added the ability to have rake tasks for multiple databases we
started looping through the configurations to collect the namespaces so
we could do `rake db:create:my_second_db`. See #32274.
This caused a problem where if you had `Rails.config.max_threads` set in
your database.yml it will blow up because the environment that defines
`max_threads` isn't loaded during `rake -T`. See #35468.
We tried to fix this by adding the ability to just load the YAML and
ignore ERB all together but that caused a bug in GitHub's YAML loading
where if you used multi-line ERB the YAML was invalid. That led us to
reverting some changes in #33748.
After trying to resolve this a bunch of ways `@tenderlove` came up with
replacing the ERB values so that we don't need to load the environment
but we also can load the YAML.
This change adds a DummyCompiler for ERB that will replace all the
values so we can load the database yaml and create the rake tasks.
Nothing else uses this method so it's "safe".
DO NOT use this method in your application.
Fixes#35468
* Add `ActiveRecord::Base.connection.truncate` for SQLite3 adapter.
SQLite doesn't support `TRUNCATE TABLE`, but SQLite3 adapter can support
`ActiveRecord::Base.connection.truncate` by using `DELETE FROM`.
`DELETE` without `WHERE` uses "The Truncate Optimization",
see https://www.sqlite.org/lang_delete.html.
* Add `rails db:seed:replant` that truncates database tables and loads the seeds
Closes#34765
Rails.autoloader and Rails.once_autoloader was just tentative API good
enough for a first patch. Rails.autoloader is singular and does not
convey in its name that there is another autoloader. That might be
confusing, for example if you set a logger and miss traces. On the other
hand, the name `once_autoloader` is very close to being horrible.
Rails.autoloaders.main and Rails.autoloaders.once read better for my
taste, and have a nice symmetry. Also, both "main" and "once" are four
letters long, short and same length.
They are tagged as "rails.main" and "rails.once", respectively.
References #35235.
- If you have hashes inside array, the hashes were getting initialized
as regular HWIA wereas we want them to be
NonSymbolAccessDeprecatedHash in order to trigger a deprecation
warning when keys are accessed with string.
This patch fixes that by overwriting the `[]=` to to the same
as what HWIA does (with the difference that we don't call
`convert_key` to not trigger a deprecation when setting value).
I also took the liberty to extract `hash.nested_under_indifferent_access`,
into a separate method to allow subclasses to return whatever
they want.
Inheriting HWIA is not common, but I think it's useful for cases
like this one where we want to preprocess reading and writing values
in the hash (for deprecation purposes or other reasons).
The assertion from the previous PR had the expected and the actual
values in the wrong order, so when a test failed the error message was
confusing.
This commit fixes the problem by switching the order.
A change to `Rails::Application.config_for` in
https://github.com/rails/rails/pull/33815 and
https://github.com/rails/rails/pull/33882 has altered the behaviour of
the returned object in a breaking manner. Before that change, nested
hashes returned from `config_for` could be accessed using non-symbol keys.
After the change, all keys are recursively symbolized so non-symbol access
fails to read the expected values.
This is a breaking change for any app that might be relying on the
nested hashes returned from `config_for` calls, and thus should be
deprecated before being removed from the codebase.
This commit introduces a temporary `NonSymbolAccessDeprecatedHash` class
that recursively wraps any nested hashes inside the `OrderedOptions`
object returned from `config_for` and issues a deprecation notice when a
non-symbol based access is performed.
This way, apps that are still relying on the ability to access these
nested hashes using non-symbol keys will be able to observe the
deprecation notices and have time to implement changes before non-symbol
access is removed for good.
A CHANGELOG entry is also added to note that non-symbol access to nested
`config_for` hashes is deprecated.
During the development of #33145, I have named a few concepts in the
code as `whitelisted`. We decided to stay away from the term and I
adjusted most of the code afterwards, but here are the cases I forgot to
change.
I also found a case in the API guide that we could have cleaned up as
well.
[ci skip]
This is a minor update to the named methods for the following:
- s/desired_capabilities/capabilities
- s/driver_options/capabilities
Since they are all the same thing we should keep the name the same
throughout the feature.
Updated docs to match / be a little bit clearer
Also updated the Gemfile for selenium-webdriver.
* There is currently no way to define specific browser capabilities since our SystemTest driver override the `option` key [Ref](a07d068078/actionpack/lib/action_dispatch/system_testing/driver.rb (L35))
This option key is used internally by selenium to add custom capabilities on the browser.
Depending on the Browser, some option are allowed to be passed inside a hash, the driver takes care of setting whatever you passed on the driver option. An example [here](a07d068078/actionpack/lib/action_dispatch/system_testing/driver.rb (L35)) where you are allowed to pass args such as `--no-sandbox` etc
However this behavior was only meant for backward compatibility and as you can see it's deprecated.
The non-deprecated behavior is to create a `<Driver>::Option` object containing all the capabilities we want. This is what we [currently do](a07d068078/actionpack/lib/action_dispatch/system_testing/browser.rb (L34-L36)) when chrome or firefox are in headless mode.
This PR allows to pass a block when calling `driven_by`, the block will be pased a `<Driver>::Option` instance. You can modify this object the way you want by adding any capabilities. The option object will be then passed to selenium.
```ruby
driven_by :selenium, using: :chrome do |driver_option|
driver_option.add_argument('--no-sandbox')
driver_option.add_emulation(device: 'iphone 4')
end
```
Rails generates `test/channels`(#34933) and
even allows `rails test:channels` (#34947).
`rails stats` has been providing info about `app/channels`,
it makes sense to add `test/channels` as well.
(I've changed test because we generate `test/channels` with some code)
Follow up to: e0d3313
- Revert renames from `encrypted` and `encrypted_file` back to `credentials`.
They might be using our Encrypted* generators but from that level of abstraction
they're still about credentials.
- Same vein: extract a `credentials` method for the `encrypted` local variable. But
don't call it `encrypted` just because it uses that under the hood. It's about
capturing the credentials. It's also useful in `change_credentials_in_system_editor`.
- Remove lots of needless argument passing. We've abstracted content_path and key_path
into methods for a reason, so they should be used. Also spares a conspicuous rename
of content_path into file_path in other methods.
- Reorders private methods so they're grouped into: command building blocks, option
parsers, and the generators.
- Extracts commonality in the credentials application tests. A tad unsure about this.
But I do like that we go with key, content thus matching the command and remove the
yield which isn't really needed.
- Moves test/credentials_test.rb to beneath the test/application directory. It's a
Rails application test, so it should be in there.
- Uses `root.join` — a neat trick gleaned from the tests! — and composes the configuration
private methods such that the building block is below the callers.
* Add option to set parallel test worker count to the physical core count of the machine
Also, use the physical core count of the machine as
the default number of workers, and generate the `test_helper.rb` file
with `parallelize(workers: :number_of_processors)`
Closes#34734
* Ensure that we always test parallel testing
Since #34734 we decided to use the physical core count of the machine as
the default number of workers in the parallel testing, we need to
ensure that some tests use at least 2 workers because we could
run those tests on VM that has only 1 physical core.
It also fixes tests failures on the CI since Travis server we are using
has only one physical core.
See https://travis-ci.org/rails/rails/jobs/469281088#L2352
The ActionDispatch::HostAuthorization is a new middleware that prevent
against DNS rebinding and other Host header attacks. By default it is
included only in the development environment with the following
configuration:
Rails.application.config.hosts = [
IPAddr.new("0.0.0.0/0"), # All IPv4 addresses.
IPAddr.new("::/0"), # All IPv6 addresses.
"localhost" # The localhost reserved domain.
]
In other environments, `Rails.application.config.hosts` is empty and no
Host header checks will be done. If you want to guard against header
attacks on production, you have to manually permit the allowed hosts
with:
Rails.application.config.hosts << "product.com"
The host of a request is checked against the hosts entries with the case
operator (#===), which lets hosts support entries of type RegExp,
Proc and IPAddr to name a few. Here is an example with a regexp.
# Allow requests from subdomains like `www.product.com` and
# `beta1.product.com`.
Rails.application.config.hosts << /.*\.product\.com/
A special case is supported that allows you to permit all sub-domains:
# Allow requests from subdomains like `www.product.com` and
# `beta1.product.com`.
Rails.application.config.hosts << ".product.com"
Sometimes `test_scaffold_tests_pass_by_default` test fails in CI.
https://travis-ci.org/rails/rails/jobs/457621750#L2095-L2120
It seems `manifest.json` was broken.
`webpacker` will compile automatically if packs is not compiled.
If parallel test is enabled, it seems that this compilation process is
executed simultaneously in multiple processes, and it may become an
inconsistent state.
In order to avoid this, compile before running the test.
DRb wraps in `DRbUnknown` if the data contains a type that can not be
resolved locally. This can happen if an error occurs in the test and the
error class can not be resolved on the server side.
When this happens, an instance of `DRbUnknown` is passed to the `result`
of `Server#record`. This causes another error(undefined method assertions
for #<DRb::DRbUnknown:> (NoMethodError)) in `reporter.record`.
This can confirm by the following steps.
```
$ rails new app -B --dev; cd app
$ rails g scaffold user name:string
$ edit `config.action_controller.allow_forgery_protection = true` in environments/test.rb
$ bin/rails t
```
In the case of `DRbUnknown` occurs, can't resolve error exception. So wrap
exception with `DRbRemoteError` in the same way as an unmarshalled object.
Addressing warnings are important but it should be out of this test scope.
https://travis-ci.org/rails/rails/jobs/454145524#L4122-L4131
```
.F
Failure:
ApplicationTests::BinSetupTest#test_bin_setup_output [test/application/bin_setup_test.rb:49]:
--- expected
+++ actual
@@ -1,4 +1,5 @@
"== Installing dependencies ==
+warning: Passing permitted_classes with the 2nd argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, permitted_classes: ...) instead.
The Gemfile's dependencies are satisfied
== Preparing database ==
rails test test/application/bin_setup_test.rb:38
```
AR instance support `filter_parameters` since #33756.
Though Regex or Proc is valid as `filter_parameters`,
they are not supported as AR#inspect.
I also add :mask option and #filter_params to
`ActiveSupport::ParameterFilter#new` to implement this.
* Use Webpacker by default on new apps
* Stop including coffee-rails by default
* Drop using a js_compressor by default
* Drop extra test for coffeescript inclusion by default
* Stick with skip_javascript to signify skipping webpack
* Don't install a JS runtime by default any more
* app/javascript will be the new default directory for JS
* Make it clear that this is just for configuring the default Webpack framework setup now
* Start using the Webpack tag in the default layout
* Irrelevant test
* jQuery is long gone
* Stop having asset pipeline compile default application.js
* Add rails-ujs by default to the Webpack setup
* Add Active Storage JavaScript to application.js pack by default
* Consistent quoting
* Add Turbolinks to default pack
* Add Action Cable to default pack
Need some work on how to set the global consumer that channels will
work with. @javan?
* Require all channels by default and use a separate consumer stub
* Channel generator now targets Webpack style
* Update task docs to match new generator style
* Use uniform import style
* Drop the JS assets generator
It was barely helpful as it was. It’s no longer helpful in a Webpacked
world. Sayonara!
* Add app/javascript to the stats directories
* Simpler import style
Which match the other imports.
* Address test failures from dropping JS compilation (and compression)
* webpacker-default: Modify `AssetsGeneratorTest`
Before:
```
$ bin/test test/generators/assets_generator_test.rb
Run options: --seed 46201
F
Failure:
AssetsGeneratorTest#test_assets [/Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/assets_generator_test.rb:12]:
Expected file "app/assets/javascripts/posts.js" to exist, but does not
bin/test /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/assets_generator_test.rb:10
.
Finished in 0.031343s, 63.8101 runs/s, 95.7152 assertions/s.
2 runs, 3 assertions, 1 failures, 0 errors, 0 skips
```
After:
```
$ bin/test test/generators/assets_generator_test.rb
Run options: --seed 43571
..
Finished in 0.030370s, 65.8545 runs/s, 65.8545 assertions/s.
2 runs, 2 assertions, 0 failures, 0 errors, 0 skips
```
* webpacker-default: Modify `ChannelGeneratorTest`
Before:
```
$ bin/test test/generators/channel_generator_test.rb
Run options: --seed 8986
.F
Failure:
ChannelGeneratorTest#test_channel_with_multiple_actions_is_created [/Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:43]:
Expected file "app/assets/javascripts/channels/chat.js" to exist, but does not
bin/test /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:34
.F
Failure:
ChannelGeneratorTest#test_channel_is_created [/Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:29]:
Expected file "app/assets/javascripts/channels/chat.js" to exist, but does not
bin/test /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:22
E
Error:
ChannelGeneratorTest#test_cable_js_is_created_if_not_present_already:
Errno::ENOENT: No such file or directory @ apply2files - /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/fixtures/tmp/app/assets/javascripts/cable.js
bin/test /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:60
F
Failure:
ChannelGeneratorTest#test_channel_suffix_is_not_duplicated [/Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:87]:
Expected file "app/assets/javascripts/channels/chat.js" to exist, but does not
bin/test /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:80
F
Failure:
ChannelGeneratorTest#test_channel_on_revoke [/Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:77]:
Expected file "app/assets/javascripts/cable.js" to exist, but does not
bin/test /Users/ttanimichi/ghq/github.com/ttanimichi/rails/railties/test/generators/channel_generator_test.rb:68
Finished in 0.064384s, 108.7227 runs/s, 481.4861 assertions/s.
7 runs, 31 assertions, 4 failures, 1 errors, 0 skips
```
After:
```
$ bin/test test/generators/channel_generator_test.rb
Run options: --seed 44857
.......
Finished in 0.060243s, 116.1961 runs/s, 697.1764 assertions/s.
7 runs, 42 assertions, 0 failures, 0 errors, 0 skips
```
* Fix shared generator tests.
* webpacker-default: Modify `ControllerGeneratorTest`
The JS assets generator was dropped. ref. 46215b1794
* Revert "Simpler import style". It's currently failing with an error of "TypeError: undefined is not an object (evaluating '__WEBPACK_IMPORTED_MODULE_2_activestorage___default.a.start')". Waiting for @javan to have a look.
This reverts commit 5d3ebb71059f635d3756cbda4ab9752027e09256.
* require webpacker in test app
* Add webpacker without making the build hang/timeout. (#33640)
* use yarn workspaces to allow for installing unreleased packages and only generate js/bootsnap when required
* no longer need to have webpacker in env templates as webpacker moved this config to yml file
* Fix rubocop violation
* Got the test passing for the running scaffold
* update expected lines of code
* update middleware tests to account for webpacker
* disable js in plugins be default to get the tests passing (#34009)
* clear codeclimate report issues
* Anything newer than currently released is good
* Use Webpacker development version during development of Rails
* Edge should get development webpacker as well
* Add changelog entry for Webpacker change
To define the attribute methods for a model, Active Record needs to know
the schema of the underlying table, which is usually achieved by making
a request to the database. This is undesirable behaviour while the app
is booting, for two reasons: it makes the boot process dependent on the
availability of the database, and it means every new process will make
one query for each table, which can cause issues for large applications.
However, if the application is using the schema cache dump feature, then
the schema cache already contains the necessary information, and we can
define the attribute methods without causing any extra database queries.
The attribute methods for a model are currently defined lazily the first
time that model is instantiated, even when `config.eager_load` is true.
This means the first request to use each model incurs the cost, which
usually involves a database round trip to fetch the schema definition.
By defining the attribute methods for all models while the application
is booting, we move that work out of any individual request. When using
a forking web server, this also reduces the number of times the schema
definition is queried by doing it once in the parent process instead of
from each forked process during their first request.
Before this change this test was passing even if we revert #31135. The
reason for that is that `app 'development'` will load the environment in
the test process and it is happening before db_create_and_drop is
called.
This was not asserting that the environment was loaded in the db:create
task itself.
To test it we enhance the db:create task with a block that writes to a
tmp file the value of the config. If the environment is loaded before
that task enhancement runs the content of the file will have "true"
insteand of "false".
If you are using the "in cache versioning" also known as "recyclable cache keys" the cache store must be aware of this scheme, otherwise you will generate cache entries that never invalidate.
This PR adds a check to the initialization process to ensure that if recyclable cache keys are being used via
```
config.active_record.cache_versioning = true
```
Then the cache store needs to show that it supports this versioning scheme. Cache stores can let Rails know that they support this scheme by adding a method `supports_in_cache_versioning?` and returning true.
In production the query cache was already being loaded before the first
request even without #33856, so added a test to make sure of it.
This new test is passing even if #33856 is reverted.
It would allow `filter_attributes` to be reused across multiple
calls to `#inspect` or `#pretty_print`.
- Add `require "set"`
- Remove `filter_attributes` instance reader. I think there is no need
to keep it.
In a test app we observed that the query cache was not enabled on the
first request. This was because the query cache hooks are installed on
load and active record is loaded in the middle of the first request.
If we remove the `on_load` from the railtie the query cache hooks will
be installed before the first request, allowing the cache to be enabled
on that first request.
This is ok because query cache doesn't load anything else, only itself
so we're not eager loading all of active record before the first
request, just the query cache hooks.
[Eileen M. Uchitelle & Matthew Draper]
