Commit Graph

115 Commits

Author SHA1 Message Date
Martin Brennan 57caf08e13
DEV: Minimal first pass of rails system test setup (#16311)
This commit introduces rails system tests run with chromedriver, selenium,
and headless chrome to our testing toolbox.

We use the `webdrivers` gem and `selenium-webdriver` which is what
the latest Rails uses so the tests run locally and in CI out of the box.

You can use `SELENIUM_VERBOSE_DRIVER_LOGS=1` to show extra
verbose logs of what selenium is doing to communicate with the system
tests.

By default JS logs are verbose so errors from JS are shown when
running system tests, you can disable this with
`SELENIUM_DISABLE_VERBOSE_JS_LOGS=1`

You can use `SELENIUM_HEADLESS=0` to run the system
tests inside a chrome browser instead of headless, which can be useful to debug things
and see what the spec sees. See note above about `bin/ember-cli` to avoid
surprises.

I have modified `bin/turbo_rspec` to exclude `spec/system` by default,
support for parallel system specs is a little shaky right now and we don't
want them slowing down the turbo by default either.

### PageObjects and System Tests

To make querying and inspecting parts of the page easier
and more reusable inbetween system tests, we are using the
concept of [PageObjects](https://www.selenium.dev/documentation/test_practices/encouraged/page_object_models/) in
our system tests. A "Page" here is generally corresponds to
an overarching ember route, e.g. "Topic" for `/t/324345/some-topic`,
and this contains logic for querying components within the topic
such as "Posts".

I have also split "Modals" into their own entity. Further down the
line we may want to explore creating independent "Component"
contexts.

Capybara DSL should be included in each PageObject class,
reference for this can be found at https://rubydoc.info/github/teamcapybara/capybara/master#the-dsl

For system tests, since they are so slow, we want to focus on
the "happy path" and not do every different possible context
and branch check using them. They are meant to be overarching
tests that check a number of things are correct using the full stack
from JS and ember to rails to ruby and then the database.

### CI Setup

Whenever a system spec fails, a screenshot
is taken and a build artifact is produced _after the entire CI run is complete_,
which can be downloaded from the Actions UI in the repo.

Most importantly, a step to build the Ember app using Ember CLI
is needed, otherwise the JS assets cannot be found by capybara:

```
- name: Build Ember CLI
  run: bin/ember-cli --build
```

A new `--build` argument has been added to `bin/ember-cli` for this
case, which is not needed locally if you already have the discourse
rails server running via `bin/ember-cli -u` since the whole server is built and
set up by default.

Co-authored-by: David Taylor <david@taylorhq.com>
2022-09-28 11:48:16 +10:00
Loïc Guitaut 0403a8633b DEV: Apply Rails 6.1 defaults
We never applied `config.load_defaults` since its inception (Rails 5.0)
and doing so is necessary to properly upgrade to all the Rails 7 new
defaults.
2022-05-24 17:13:44 +02:00
Alan Guo Xiang Tan fd1dc91eed
DEV: Don't cache watched words in test env (#16731)
The cache was causing state to leak between tests since the `WatchedWord` record in the DB would have been rolled back but `WordWatcher` still had the word in the cache.
2022-05-12 14:45:05 +08:00
Jarek Radosz fb1a3a1dbb
DEV: Drop `TrackingLogger` for `FakeLogger` (#16642) 2022-05-05 09:50:43 +08:00
Gerhard Schlager 8442a07c13
DEV: Compatibility with TruffleRuby (#16641) 2022-05-05 09:50:02 +08:00
Loïc Guitaut 008b700a3f DEV: Upgrade to Rails 7
This patch upgrades Rails to version 7.0.2.4.
2022-04-28 11:51:03 +02:00
Martin Brennan 3e4621c2cb
FEATURE: Polymorphic bookmarks pt. 2 (lists, search) (#16335)
This pull request follows on from https://github.com/discourse/discourse/pull/16308. This one does the following:

* Changes `BookmarkQuery` to allow for querying more than just Post and Topic bookmarkables
* Introduces a `Bookmark.register_bookmarkable` method which requires a model, serializer, fields and preload includes for searching. These registered `Bookmarkable` types are then used when validating new bookmarks, and also when determining which serializer to use for the bookmark list. The `Post` and `Topic` bookmarkables are registered by default.
* Adds new specific types for Post and Topic bookmark serializers along with preloading of associations in `UserBookmarkList`
* Changes to the user bookmark list template to allow for more generic bookmarkable types alongside the Post and Topic ones which need to display in a particular way

All of these changes are gated behind the `use_polymorphic_bookmarks` site setting, apart from the .hbs changes where I have updated the original `UserBookmarkSerializer` with some stub methods.

Following this PR will be several plugin PRs (for assign, chat, encrypt) that will register their own bookmarkable types or otherwise alter the bookmark serializers in their own way, also gated behind `use_polymorphic_bookmarks`.

This commit also removes `BookmarkQuery.preloaded_custom_fields` and the functionality surrounding it. It was added in 0cd502a558 but only used by one plugin (discourse-assign) where it has since been removed, and is now used by no plugins. We don't need it anymore.
2022-04-22 08:23:42 +10:00
Sam d4d3580761
PERF: perform all cached counting in background (#15991)
Previously cached counting made redis calls in main thread and performed
the flush in main thread.

This could lead to pathological states in extreme heavy load.

This refactor reduces load and cleans up the interface
2022-02-22 16:45:25 +00:00
Jarek Radosz aaf432df86
DEV: Remove mock_redis (#15985)
Was used just in one spec file. And we prefer to run specs against a real redis server.
2022-02-18 01:14:38 +01:00
Jarek Radosz 5a50f18c0c
DEV: Avoid `$` globals (#15453)
Also:
* Remove an unused method (#fill_email)
* Replace a method that was used just once (#generate_username) with `SecureRandom.alphanumeric`
* Remove an obsolete dev puma `tmp/restart` file logic
2022-01-08 23:39:46 +01:00
Peter Zhu c5fd8c42db
DEV: Fix methods removed in Ruby 3.2 (#15459)
* File.exists? is deprecated and removed in Ruby 3.2 in favor of
File.exist?
* Dir.exists? is deprecated and removed in Ruby 3.2 in favor of
Dir.exist?
2022-01-05 18:45:08 +01:00
David Taylor 08f4edc032
DEV: Reset OmniAuth test mode between tests (#15278)
OmniAuth test mode is disabled by default, so that we can integration-test the omniauth strategies. Sometimes, we manually enable test mode for specific specs. This commit ensures that test_mode is always disabled again after each spec.
2021-12-13 17:05:26 +00:00
Daniel Waterworth 02245ce41f
PERF: Redis snapshotting during tests (#15260)
We can fake redis transactions so that `fab!` works for redis and PG
data, but it's too slow to be used indiscriminately. Instead, you can
opt into it with the `use_redis_snapshotting` helper.

Insofar as snapshotting allows us to `fab!` more things, it provides a
speedup.
2021-12-10 14:25:26 -06:00
Daniel Waterworth 95c75d3f4f
DEV: Set uploads_id_seq starting point before tests (#15215)
Some tests don't pass when this is elevated. They should be fixed,
since, at some point, we may create enough uploads during tests that
they fail naturally.
2021-12-07 12:26:35 -06:00
Alan Guo Xiang Tan 0edacbd8f7
DEV: Load fabricators for plugins automatically. (#15106) 2021-11-30 15:55:45 +11:00
Osama Sayegh b86127ad12
FEATURE: Apply rate limits per user instead of IP for trusted users (#14706)
Currently, Discourse rate limits all incoming requests by the IP address they
originate from regardless of the user making the request. This can be
frustrating if there are multiple users using Discourse simultaneously while
sharing the same IP address (e.g. employees in an office).

This commit implements a new feature to make Discourse apply rate limits by
user id rather than IP address for users at or higher than the configured trust
level (1 is the default).

For example, let's say a Discourse instance is configured to allow 200 requests
per minute per IP address, and we have 10 users at trust level 4 using
Discourse simultaneously from the same IP address. Before this feature, the 10
users could only make a total of 200 requests per minute before they got rate
limited. But with the new feature, each user is allowed to make 200 requests
per minute because the rate limits are applied on user id rather than the IP
address.

The minimum trust level for applying user-id-based rate limits can be
configured by the `skip_per_ip_rate_limit_trust_level` global setting. The
default is 1, but it can be changed by either adding the
`DISCOURSE_SKIP_PER_IP_RATE_LIMIT_TRUST_LEVEL` environment variable with the
desired value to your `app.yml`, or changing the setting's value in the
`discourse.conf` file.

Requests made with API keys are still rate limited by IP address and the
relevant global settings that control API keys rate limits.

Before this commit, Discourse's auth cookie (`_t`) was simply a 32 characters
string that Discourse used to lookup the current user from the database and the
cookie contained no additional information about the user. However, we had to
change the cookie content in this commit so we could identify the user from the
cookie without making a database query before the rate limits logic and avoid
introducing a bottleneck on busy sites.

Besides the 32 characters auth token, the cookie now includes the user id,
trust level and the cookie's generation date, and we encrypt/sign the cookie to
prevent tampering.

Internal ticket number: t54739.
2021-11-17 23:27:30 +03:00
Martin Brennan e0be6ce1ee
DEV: Catch Mocha::ExpectationError in request specs (#14897)
Same issue as 28b00dc6fc, the
Mocha::ExpectationError inherits from Exception instead
of StandardError so RspecErrorTracker does not show the
actual failed expectation in request specs, the status of
the response is just 500 with no further detail.
2021-11-12 13:10:16 +10:00
David Taylor 13fdc979a8
DEV: Improve multisite testing (#14884)
This commit adds the RailsMultisite middleware in test mode when Rails.configuration.multisite is true. This allows for much more realistic integration testing. The `multisite_spec.rb` file is rewritten to avoid needing to simulate a middleware stack.
2021-11-11 16:44:58 +00:00
Martin Brennan 28b00dc6fc
DEV: Output webmock errors in request specs (#14782)
* DEV: Output webmock errors in request specs

In request specs, if you had not properly mocked an external
HTTP call, you would end up with a 500 error with no further
information instead of your expected response code, with an
rspec output like this:

```
Failures:

  1) UploadsController#generate_presigned_put when the store is external generates a presigned URL and creates an external upload stub
     Failure/Error: expect(response.status).to eq(200)

       expected: 200
            got: 500

       (compared using ==)
     # ./spec/requests/uploads_controller_spec.rb:727:in `block (4 levels) in <top (required)>'
     # ./spec/rails_helper.rb:280:in `block (2 levels) in <top (required)>'
```

This is not helpful at all when you want to find what you actually
failed to mock, which is shown straight away in non-request specs.

This commit introduces a rescue_from block in the application
controller to log this error, so we have a much nicer output that
helps the developer find the issue:

```
Failures:

  1) UploadsController#generate_presigned_put when the store is external generates a presigned URL and creates an external upload stub
     Failure/Error: expect(response.status).to eq(200)

       expected: 200
            got: 500

       (compared using ==)
     # ./spec/requests/uploads_controller_spec.rb:727:in `block (4 levels) in <top (required)>'
     # ./spec/rails_helper.rb:280:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # WebMock::NetConnectNotAllowedError:
     #   Real HTTP connections are disabled. Unregistered request: GET https://s3-upload-bucket.s3.us-west-1.amazonaws.com/?cors with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'', 'Authorization'=>'AWS4-HMAC-SHA256 Credential=some key/20211101/us-west-1/s3/aws4_request, SignedHeaders=host;user-agent;x-amz-content-sha256;x-amz-date, Signature=test', 'Host'=>'s3-upload-bucket.s3.us-west-1.amazonaws.com', 'User-Agent'=>'aws-sdk-ruby3/3.121.2 ruby/2.7.1 x86_64-linux aws-sdk-s3/1.96.1', 'X-Amz-Content-Sha256'=>'test', 'X-Amz-Date'=>'20211101T035113Z'}
     #
     #   You can stub this request with the following snippet:
     #
     #   stub_request(:get, "https://s3-upload-bucket.s3.us-west-1.amazonaws.com/?cors").
     #     with(
     #       headers: {
     #   	  'Accept'=>'*/*',
     #   	  'Accept-Encoding'=>'',
     #   	  'Authorization'=>'AWS4-HMAC-SHA256 Credential=some key/20211101/us-west-1/s3/aws4_request, SignedHeaders=host;user-agent;x-amz-content-sha256;x-amz-date, Signature=test',
     #   	  'Host'=>'s3-upload-bucket.s3.us-west-1.amazonaws.com',
     #   	  'User-Agent'=>'aws-sdk-ruby3/3.121.2 ruby/2.7.1 x86_64-linux aws-sdk-s3/1.96.1',
     #   	  'X-Amz-Content-Sha256'=>'test',
     #   	  'X-Amz-Date'=>'20211101T035113Z'
     #       }).
     #     to_return(status: 200, body: "", headers: {})
     #
     #   registered request stubs:
     #
     #   stub_request(:head, "https://s3-upload-bucket.s3.us-west-1.amazonaws.com/")
     #
     #   ============================================================
```

* DEV: Require webmock in application controller if rails.env.test

* DEV: Rescue from StandardError and NetConnectNotAllowedError
2021-11-01 14:38:41 +08:00
Arpit Jalan d1fc759ac4
FIX: remove 'crawl_images' site setting (#14646) 2021-10-19 17:12:29 +05:30
Martin Brennan 93af139a76
Revert "DEV: Move rate limiter disable to after :each for tests (#13986)" (#13987)
This reverts commit 52035dc665.
2021-08-10 14:12:36 +10:00
Martin Brennan 52035dc665
DEV: Move rate limiter disable to after :each for tests (#13986)
We weren't calling clear_all! for the rate limiter which
was the first problem, and the second problem was that it
is very odd to do state cleanup before tests instead of after,
so moved the disabling and clear_all! to after.
2021-08-10 13:38:35 +10:00
Arpit Jalan 283b08d45f
DEV: Absorb onebox gem into core (#12979)
* Move onebox gem in core library

* Update template file path

* Remove warning for onebox gem caching

* Remove onebox version file

* Remove onebox gem

* Add sanitize gem

* Require onebox library in lazy-yt plugin

* Remove onebox web specific code

This code was used in standalone onebox Sinatra application

* Merge Discourse specific AllowlistedGenericOnebox engine in core

* Fix onebox engine filenames to match class name casing

* Move onebox specs from gem into core

* DEV: Rename `response` helper to `onebox_response`

Fixes a naming collision.

* Require rails_helper

* Don't use `before/after(:all)`

* Whitespace

* Remove fakeweb

* Remove poor unit tests

* DEV: Re-add fakeweb, plugins are using it

* Move onebox helpers

* Stub Instagram API

* FIX: Follow additional redirect status codes (#476)

Don’t throw errors if we encounter 303, 307 or 308 HTTP status codes in responses

* Remove an empty file

* DEV: Update the license file

Using the copy from https://choosealicense.com/licenses/gpl-2.0/#

Hopefully this will enable GitHub to show the license UI?

* DEV: Update embedded copyrights

* DEV: Add Onebox copyright notice

* DEV: Add MIT license, convert COPYRIGHT.txt to md

* DEV: Remove an incorrect copyright claim

Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Co-authored-by: jbrw <jamie@goatforce5.org>
2021-05-26 15:11:35 +05:30
Josh Soref 59097b207f
DEV: Correct typos and spelling mistakes (#12812)
Over the years we accrued many spelling mistakes in the code base. 

This PR attempts to fix spelling mistakes and typos in all areas of the code that are extremely safe to change 

- comments
- test descriptions
- other low risk areas
2021-05-21 11:43:47 +10:00
Joffrey JAFFEUX f3a4c12564
DEV: removes unused set_env (#12961) 2021-05-06 13:04:18 +02:00
Dan Ungureanu c89f97aafe
DEV: Better SimpleCov configuration (#12544)
It makes SimpleCov work with turbo_rspec and uses the default Rails
configuration (with some changes) to groups files by their type
(models, controllers, etc).
2021-03-29 21:01:35 +03:00
David Taylor ed91385c18
DEV: Update `DB.after_commit` to be compatible with 'real' transactions (#11294)
Previously it matched the behavior of standard ActiveRecord after_commit callbacks. They do not work well within `joinable: false` nested transactions. Now `DB.after_commit` callbacks will only be run when the outermost transaction has been committed.

Tests always run inside transactions, so this also introduces some logic to run callbacks once the test-wrapping transaction is reached.
2020-12-08 11:03:31 +11:00
David Taylor 803b8933fa
DEV: Ensure DiscourseEvent handlers cleaned up during specs (#11205) 2020-11-11 19:46:13 +00:00
Jarek Radosz e00abbe1b7 DEV: Clean up S3 specs, stubs, and helpers
Extracted commonly used spec helpers into spec/support/uploads_helpers.rb, removed unused stubs and let definitions. Makes it easier to write new S3-related specs without copy and pasting setup steps from other specs.
2020-09-28 12:02:25 +01:00
Gerhard Schlager 11647b79f7 FIX: SiteSettings::LocalProcessProvider didn't work on multisite
It always used "test" as current site.
2020-08-20 11:15:20 +02:00
Guo Xiang Tan c6202af005
Update rubocop to 2.3.1. 2020-07-24 17:19:21 +08:00
Martin Brennan b62ad003ce Always require pry gems in rails_helper
* remove IMPROVED_SPEC_DEBUGGING env var guard
2020-06-17 10:08:53 +10:00
Martin Brennan e84ffb4861
DEV: Add pry-byebug and optionally require pry behind a ENV var for better spec debugging (#9984)
We removed pry-nav a while back because it is not up to date with pry but it is super useful. Luckily pry-byebug is here to save us all from Satan's power.

To get this to work you need to add the following to your $HOME/.pryrc file.

```
if defined?(PryByebug)
  Pry.commands.alias_command 'c', 'continue'
  Pry.commands.alias_command 's', 'step'
  Pry.commands.alias_command 'n', 'next'
  Pry.commands.alias_command 'f', 'finish'
end

Pry::Commands.command /^$/, "repeat last command" do
  pry_instance.run_command Pry.history.to_a.last
end
```

The require-ing of pry, pry-rails, and pry-byebug in specs is controlled by the IMPROVED_SPEC_DEBUGGING flag (disabled by default).
2020-06-05 12:30:34 +10:00
Michael Brown d9a02d1336
Revert "Revert "Merge branch 'master' of https://github.com/discourse/discourse""
This reverts commit 20780a1eee.

* SECURITY: re-adds accidentally reverted commit:
  03d26cd6: ensure embed_url contains valid http(s) uri
* when the merge commit e62a85cf was reverted, git chose the 2660c2e2 parent to land on
  instead of the 03d26cd6 parent (which contains security fixes)
2020-05-23 00:56:13 -04:00
Jeff Atwood 20780a1eee Revert "Merge branch 'master' of https://github.com/discourse/discourse"
This reverts commit e62a85cf6f, reversing
changes made to 2660c2e21d.
2020-05-22 20:25:56 -07:00
Guo Xiang Tan 68db5deaec
FIX: Badge granter was disabled by default. 2020-05-21 06:55:22 +08:00
Robin Ward 096eca0ee8 FIX: Requests were not being logged correctly
`enable` was defaulting to `nil` which is not what we wanted.
2020-05-20 15:09:17 -04:00
Krzysztof Kotlarek f99f6ca111
FIX: randomize file name when created from fixtures (#9731)
* FIX: randomize file name when created from fixtures

When a temporary file is created from fixtures it should have a unique name.
It is to prevent a collision in parallel specs evaluation

* FIX: use /tmp/pid folder to keep fixture files
2020-05-19 09:09:36 +10:00
David Taylor d4717f5d1e
DEV: Reduce repeated code in discourse_plugin_registry (#9705)
Use a helper method to simplify creating a new register. Previously this would require creating lots of different methods manually, and adding every register to the clear/reset functions
2020-05-13 12:25:34 +01:00
Daniel Waterworth 7876ee2d67 DEV: upgrade Rails
Latest version of Rails contains compatibility fixes for Ruby 2.7 and some
minor security fixes we would like to have

It also broke some of the multisite tests.

Rails tries to use the same connection for reading from a replica as writing
to the leader during tests, because, with everything happening in a
transaction, changes to the DB wouldn't otherwise be reflected in the
replica connection.

The difference now is that Rails tries to do this for connections opened
after the test has started which affected rails multisite connections.

The upshot of this is that, as things stand, you are likely to
experience problems if you try to connect to a different multisite DB in
a test when the `current_db` is not 'default'.
2020-04-20 12:55:53 +01:00
Sam Saffron 0375a5ac0b
DEV: reduce logging when no external id is specified
Previously we were returning an unknown sso error and logging a message
when external id was blank. This noise is not needed.
2020-04-08 12:42:28 +10:00
Jarek Radosz d21d80198c
DEV: Update rubocop-discourse (#9270)
Includes:
* DEV: Use `eq_time` matcher
2020-03-26 16:32:41 +01:00
Jarek Radosz 29b35aa64c
DEV: Improve flaky time-sensitive specs (#9141) 2020-03-10 22:13:17 +01:00
Martin Brennan 66f2db4ea4 SECURITY: 2FA with U2F / TOTP 2020-01-15 11:27:12 +01:00
Martin Brennan cb660ef952 SECURITY: Improve second factor auth logic 2020-01-10 10:45:56 +10:00
David Taylor f47909a2b6 DEV: Check for pending migrations before running rspec suite
This should catch a few scenarios which can waste a lot of time in development
 - Forgetting to run migrations
 - Missing plugin migrations
 - Missing post_deploy migrations
2019-12-13 12:50:18 +00:00
Joffrey JAFFEUX 0d3d2c43a0
DEV: s/\$redis/Discourse\.redis (#8431)
This commit also adds a rubocop rule to prevent global variables.
2019-12-03 10:05:53 +01:00
Krzysztof Kotlarek 427d54b2b0 DEV: Upgrading Discourse to Zeitwerk (#8098)
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains. 

We no longer need to use Rails "require_dependency" anywhere and instead can just use standard 
Ruby patterns to require files.

This is a far reaching change and we expect some followups here.
2019-10-02 14:01:53 +10:00
Daniel Waterworth 15c02c03c7 DEV: Split out multisite tests in bin/turbo_rspec
* A new process is started that just runs the multisite tests
 * The other processes are instructed to exclude the multisite tests
2019-08-29 11:47:58 +01:00
Daniel Waterworth 23dd50316c FIX: Rollback when multisite tests raise exceptions 2019-08-01 15:26:06 +01:00