Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Promote Dummy app's Message to Active Record #248

Merged
merged 1 commit into from
Sep 23, 2021

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Sep 23, 2021

In an effort to reduce the number of flaky tests and the amount of
in-memory state leaking between tests, this commit promotes the test
suite's dummy application's Message object to an ApplicationRecord
descendant.

Something in the test suite configuration is preventing the database
from being wiped between test runs. This results in state leaking
between tests. As a result, our Continuous Integration tests are flaky.

This commit introduces a short-term remedy by adding a setup { Message.delete_all } block to the test/system/broadcasts_test.rb.
This should be unnecessary, but until we determine to root cause, it
helps pass the suite more consistently.

In an effort to reduce the number of flaky tests and the amount of
in-memory state leaking between tests, this commit promotes the test
suite's dummy application's `Message` object to an `ApplicationRecord`
descendant.

Something in the test suite configuration is preventing the database
from being wiped between test runs. This results in state leaking
between tests. As a result, our Continuous Integration tests are flaky.

This commit introduces a short-term remedy by adding a `setup {
Message.delete_all }` block to the `test/system/broadcasts_test.rb`.
This should be unnecessary, but until we determine to root cause, it
helps pass the suite more consistently.
@dhh dhh merged commit c2dc5b1 into hotwired:main Sep 23, 2021
@seanpdoyle seanpdoyle deleted the message-active-record-tests branch September 23, 2021 17:58
seanpdoyle added a commit to seanpdoyle/turbo-rails that referenced this pull request May 3, 2022
> Something in the test suite configuration is preventing the database
> from being wiped between test runs. This results in state leaking
> between tests. As a result, our Continuous Integration tests are flaky.
>
> - [hotwired#248][]

As a follow-up to the [short-term solution][] shipped in
[hotwired#248][], this commit attempts to make the
`test/turbo_test.rb` file's setup consistent with the test harness setup
generated by Rails' [engine generator][] code.

To that end, this commit:

* renames the `test/turbo_test.rb` file to `test/test_helper.rb`
* omits one-off `require` calls for particular dependencies
* re-orders the require calls so that the
  `../test/dummy/config/environment` file is required ahead of the
  `rails/test_help` file

[engine generator]: https://github.com/rails/rails/blob/3c48b4030adbded21bebaa0d78254216cca48a6e/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt
[hotwired#248]: hotwired#248
[short-term solution]: hotwired@c2dc5b1
seanpdoyle added a commit to seanpdoyle/turbo-rails that referenced this pull request May 23, 2022
> Something in the test suite configuration is preventing the database
> from being wiped between test runs. This results in state leaking
> between tests. As a result, our Continuous Integration tests are flaky.
>
> - [hotwired#248][]

As a follow-up to the [short-term solution][] shipped in
[hotwired#248][], this commit attempts to make the
`test/turbo_test.rb` file's setup consistent with the test harness setup
generated by Rails' [engine generator][] code.

To that end, this commit:

* renames the `test/turbo_test.rb` file to `test/test_helper.rb`
* omits one-off `require` calls for particular dependencies
* re-orders the require calls so that the
  `../test/dummy/config/environment` file is required ahead of the
  `rails/test_help` file

[engine generator]: https://github.com/rails/rails/blob/3c48b4030adbded21bebaa0d78254216cca48a6e/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt
[hotwired#248]: hotwired#248
[short-term solution]: hotwired@c2dc5b1
seanpdoyle added a commit to seanpdoyle/turbo-rails that referenced this pull request Aug 1, 2022
> Something in the test suite configuration is preventing the database
> from being wiped between test runs. This results in state leaking
> between tests. As a result, our Continuous Integration tests are flaky.
>
> - [hotwired#248][]

As a follow-up to the [short-term solution][] shipped in
[hotwired#248][], this commit attempts to make the
`test/turbo_test.rb` file's setup consistent with the test harness setup
generated by Rails' [engine generator][] code.

To that end, this commit:

* renames the `test/turbo_test.rb` file to `test/test_helper.rb`
* omits one-off `require` calls for particular dependencies
* re-orders the require calls so that the
  `../test/dummy/config/environment` file is required ahead of the
  `rails/test_help` file

[engine generator]: https://github.com/rails/rails/blob/3c48b4030adbded21bebaa0d78254216cca48a6e/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt
[hotwired#248]: hotwired#248
[short-term solution]: hotwired@c2dc5b1
dhh pushed a commit that referenced this pull request Aug 2, 2022
* Make `turbo_test.rb` with Rails' generated `test_helper.rb`

> Something in the test suite configuration is preventing the database
> from being wiped between test runs. This results in state leaking
> between tests. As a result, our Continuous Integration tests are flaky.
>
> - [#248][]

As a follow-up to the [short-term solution][] shipped in
[#248][], this commit attempts to make the
`test/turbo_test.rb` file's setup consistent with the test harness setup
generated by Rails' [engine generator][] code.

To that end, this commit:

* renames the `test/turbo_test.rb` file to `test/test_helper.rb`
* omits one-off `require` calls for particular dependencies
* re-orders the require calls so that the
  `../test/dummy/config/environment` file is required ahead of the
  `rails/test_help` file

[engine generator]: https://github.com/rails/rails/blob/3c48b4030adbded21bebaa0d78254216cca48a6e/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt
[#248]: #248
[short-term solution]: c2dc5b1

* Use Ruby 2.7 argument forwarding

Switching to argument forwarding addresses deprecation warnings like:

```
hotwired/turbo-rails/test/streams/streams_channel_test.rb:140: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
hotwired/turbo-rails/test/turbo_test.rb:14: warning: The called method `render' is defined here
```

* Tests: Load 6.1 defaults in Dummy Application

Resolve deprecation warnings like:

```
Preparing test database
DEPRECATION WARNING: Non-URL-safe CSRF tokens are deprecated. Use 6.1 defaults or above. (called from <top (required)> at /home/runner/work/turbo-rails/turbo-rails/test/dummy/config/initializers/inspect_helpers.rb:1)
DEPRECATION WARNING: Using legacy connection handling is deprecated. Please set
`legacy_connection_handling` to `false` in your application.

The new connection handling does not support `connection_handlers`
getter and setter.

Read more about how to migrate at: https://guides.rubyonrails.org/active_record_multiple_databases.html#migrate-to-the-new-connection-handling
 (called from <top (required)> at /home/runner/work/turbo-rails/turbo-rails/test/test_helper.rb:6)
```

Since our GitHub CI matrix includes `6.1`, `7.0`, and `main`, CI's tests
should run with at least the `6.1` defaults.

* CI: Continue other executions on error

Remove the `continue-on-error` configuration and instead allow other
jobs complete in spite of failures.

* Improve Flaky Test: Clear fields before filling in

Resolve a flaky System Test by ensuring that an input is clear before
filling in a new value.

* Improve flaky tests: Broadcasts

First, don't render HTML with the `<turbo-stream-source>` element.
Instead, append the element when clicking a `<button>`.
atosbucket added a commit to atosbucket/turbo-rails that referenced this pull request Apr 11, 2024
* Make `turbo_test.rb` with Rails' generated `test_helper.rb`

> Something in the test suite configuration is preventing the database
> from being wiped between test runs. This results in state leaking
> between tests. As a result, our Continuous Integration tests are flaky.
>
> - [hotwired/turbo-rails#248][]

As a follow-up to the [short-term solution][] shipped in
[hotwired/turbo-rails#248][], this commit attempts to make the
`test/turbo_test.rb` file's setup consistent with the test harness setup
generated by Rails' [engine generator][] code.

To that end, this commit:

* renames the `test/turbo_test.rb` file to `test/test_helper.rb`
* omits one-off `require` calls for particular dependencies
* re-orders the require calls so that the
  `../test/dummy/config/environment` file is required ahead of the
  `rails/test_help` file

[engine generator]: https://github.com/rails/rails/blob/3c48b4030adbded21bebaa0d78254216cca48a6e/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt
[hotwired/turbo-rails#248]: hotwired/turbo-rails#248
[short-term solution]: hotwired/turbo-rails@c2dc5b1

* Use Ruby 2.7 argument forwarding

Switching to argument forwarding addresses deprecation warnings like:

```
hotwired/turbo-rails/test/streams/streams_channel_test.rb:140: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
hotwired/turbo-rails/test/turbo_test.rb:14: warning: The called method `render' is defined here
```

* Tests: Load 6.1 defaults in Dummy Application

Resolve deprecation warnings like:

```
Preparing test database
DEPRECATION WARNING: Non-URL-safe CSRF tokens are deprecated. Use 6.1 defaults or above. (called from <top (required)> at /home/runner/work/turbo-rails/turbo-rails/test/dummy/config/initializers/inspect_helpers.rb:1)
DEPRECATION WARNING: Using legacy connection handling is deprecated. Please set
`legacy_connection_handling` to `false` in your application.

The new connection handling does not support `connection_handlers`
getter and setter.

Read more about how to migrate at: https://guides.rubyonrails.org/active_record_multiple_databases.html#migrate-to-the-new-connection-handling
 (called from <top (required)> at /home/runner/work/turbo-rails/turbo-rails/test/test_helper.rb:6)
```

Since our GitHub CI matrix includes `6.1`, `7.0`, and `main`, CI's tests
should run with at least the `6.1` defaults.

* CI: Continue other executions on error

Remove the `continue-on-error` configuration and instead allow other
jobs complete in spite of failures.

* Improve Flaky Test: Clear fields before filling in

Resolve a flaky System Test by ensuring that an input is clear before
filling in a new value.

* Improve flaky tests: Broadcasts

First, don't render HTML with the `<turbo-stream-source>` element.
Instead, append the element when clicking a `<button>`.
luisjose1996 added a commit to luisjose1996/Turbo-Rails that referenced this pull request May 10, 2024
* Make `turbo_test.rb` with Rails' generated `test_helper.rb`

> Something in the test suite configuration is preventing the database
> from being wiped between test runs. This results in state leaking
> between tests. As a result, our Continuous Integration tests are flaky.
>
> - [hotwired/turbo-rails#248][]

As a follow-up to the [short-term solution][] shipped in
[hotwired/turbo-rails#248][], this commit attempts to make the
`test/turbo_test.rb` file's setup consistent with the test harness setup
generated by Rails' [engine generator][] code.

To that end, this commit:

* renames the `test/turbo_test.rb` file to `test/test_helper.rb`
* omits one-off `require` calls for particular dependencies
* re-orders the require calls so that the
  `../test/dummy/config/environment` file is required ahead of the
  `rails/test_help` file

[engine generator]: https://github.com/rails/rails/blob/3c48b4030adbded21bebaa0d78254216cca48a6e/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt
[hotwired/turbo-rails#248]: hotwired/turbo-rails#248
[short-term solution]: hotwired/turbo-rails@c2dc5b1

* Use Ruby 2.7 argument forwarding

Switching to argument forwarding addresses deprecation warnings like:

```
hotwired/turbo-rails/test/streams/streams_channel_test.rb:140: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
hotwired/turbo-rails/test/turbo_test.rb:14: warning: The called method `render' is defined here
```

* Tests: Load 6.1 defaults in Dummy Application

Resolve deprecation warnings like:

```
Preparing test database
DEPRECATION WARNING: Non-URL-safe CSRF tokens are deprecated. Use 6.1 defaults or above. (called from <top (required)> at /home/runner/work/turbo-rails/turbo-rails/test/dummy/config/initializers/inspect_helpers.rb:1)
DEPRECATION WARNING: Using legacy connection handling is deprecated. Please set
`legacy_connection_handling` to `false` in your application.

The new connection handling does not support `connection_handlers`
getter and setter.

Read more about how to migrate at: https://guides.rubyonrails.org/active_record_multiple_databases.html#migrate-to-the-new-connection-handling
 (called from <top (required)> at /home/runner/work/turbo-rails/turbo-rails/test/test_helper.rb:6)
```

Since our GitHub CI matrix includes `6.1`, `7.0`, and `main`, CI's tests
should run with at least the `6.1` defaults.

* CI: Continue other executions on error

Remove the `continue-on-error` configuration and instead allow other
jobs complete in spite of failures.

* Improve Flaky Test: Clear fields before filling in

Resolve a flaky System Test by ensuring that an input is clear before
filling in a new value.

* Improve flaky tests: Broadcasts

First, don't render HTML with the `<turbo-stream-source>` element.
Instead, append the element when clicking a `<button>`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants