docker.io rate limits _should_ be addressed now. Switch back to
`debian-12` runners.
This is now a conditional within the `tests` workflow as we evaluate the
migration.
This reverts commit a99e2c62e6.
The self-hosted Github runners have been provisioned, and we can switch
to using them for evaluation.
To prefer Github-hosted runners, you can safely revert this commit.
See: t/123181.
This will bring significant improvements to install speed & storage requirements. For information on how it may affect you, see https://meta.discourse.org/t/324521
This commit:
- removes the `yarn.lock` and replaces with `pnpm-lock.yaml`
- updates workspaces to pnpm format
- adjusts package dependencies to work with pnpm's stricter resolution strategy
- updates Rails app to load modules from more specific node_modules directories
- adds a `.pnpmfile` which automatically cleans up old yarn-managed `node_modules` directories
- updates various scripts to call `pnpm` instead of `yarn`
- updates patches to use pnpm's native patch system instead of patch-package
- adds a patch for licensee to support pnpm
This regressed in b83a2a34a4 because the
Github actions docs doesn't make it clear that `runner.name` is actually
the runner's name plus some unique string appended at the end. Why they
would do that is beyond me.
We cannot just key on `runner.os` the number of CPU cores matter as
well. Therefore, we need to key on `runner.name` instead since each
runner has its own unique OS and CPU cores. Technically, two different
runner with different names can have the same `os` and `cpu cores` but
we don't have that problem now.
QUnit tests are failing in different ways on Chromium in Debian
bookworm. We have no interest in figuring out why as it is not a good
use of our time and the long term plan is to switch to Chrome for Testing
anyway.
Previously "themes frontend" CI job would:
1. pull compatible versions of themes that happened to be in the base image
2. clone all official themes (overriding the compatible versions from 1.)
3. run tests
`discourse-ai` has custom gems which need to be bumped in order to be
compatible with Ruby 3.3. However, its version is pinned so we can't
pull in the commits in which upgrades the gems to be compatible with
Ruby 3.3. Just avoid running the specs on `stable` branch for now until
we release a new stable.
This commit adds a step in our tests workflow on Github actions to update the themes to
use the compatible version when not running aginast the `main` branch.
This is to ensure that we are not running
the tests for themes against an incompatible version of Discourse.
Why this change?
Bundle cache should be keyed on ruby version as well as the debian
release name. Changes to the debian release can affect the way gems are
installed since gems may link to different versions of binaries.
The version of Chromium we have in our images (120) is relatively
unstable and our system specs break regularly.
This patch makes sure Chrome is used instead for system specs.
Why this change?
We run on different runners depending on the scenario. We should use the
right number of parallel jobs for bundle install based on the number of
CPU cores the runner has.
This is so the CI output on GitHub actions isn't showing
tons and tons of unnecessary log data every time you want
to see the important thing, which is the actual test failure.
Why this change?
The output is too verbose and prevents us from quickly identifying tests
failures. Now that our tests are way more stable and less flaky, we can
drop the documentation format since we do not need it for debugging
purposes that often anymore
Why this change?
Our tests are more stable these days and there is little to no need for
us to be retrying on PRs which helps to increase confidence in our test
suite since flaky tests are raised earlier.
Why this change?
This regressed in 6e9fbb5bab because we
had a `request.xhr?` check before we decide to block requests. However,
there could not none-xhr requests which we need to block as well at the
end of each system test when `@@block_requests` is true.
This also reverts commit 6437f27f90.
Why this change?
On CI, we have been seeing flaky system tests because ActiveRecord is
unable to checkout a connection. This patch is meant to help us debug
which thread is not returning the connection to the queue.
Example of timeout issue: https://github.com/discourse/discourse/actions/runs/8012541636/job/21888013082
Why this change?
On CI, we have been seeing flaky system tests because ActiveRecord is
unable to checkout a connection. This patch is meant to help us debug
which thread is not returning the connection to the queue.
Why this change?
We have been seeing checkout timeouts happening on CI when using the
default of 5 seconds. This can happen in system tests when the server
has to process many requests using the same database connection.
Therefore, we will double the timeout for now and monitor if stuff
continues to timeout.
Why this change?
I have been investigating transaction related issues with our system
tests and I have a hard time figuring out what is causing the problem.
To help simplify our environment further, we will set the pool size in
the test environment to 1 so that it is impossible for us to be fetching
a different connection between the threads since they all share the
connection pool.
Also set `reaping_frequency` to `0` to ensure we don't reap any
connection ensuring the same connection is always used.
Why this change?
In https://www.postgresql.org/docs/current/non-durability.html, it is
recommended to create unlogged tables to avoid WAL writes which can help
speed at performance at the expense of durability. In the CI env, there is no need for durability at all.
Therefore, we are going to be creating unlogged tables by default.
Co-authored-by: Ted Johansson <ted@discourse.org>
Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>
Why this change?
We have been looking into a flaky system tests in one of our plugins
where the DB transaction flow can be messed up from time to time. Our
debugging effort is complicated by that fact that `test-prof` starts a
DB transaction in a `before(:all)` block which makes it hard to properly
log information. By allowing test-prof to be disabled completely, we
believe it will make it easier for us to isolate the problem we are
investigating.
What does this change do?
1. Avoid loading test-prof files if `PREFABRICATION` env has been set to
`0`.
2. Set `PREFABRICATION=0` for plugin system tests in Github actions
Why this change?
When running system tests on our CI, we have been occasionally seeing
server errors like:
```
Error encountered while proccessing /stylesheets/desktop_e58cf7f686aab173f9b778797f241913c2833c39.css
NoMethodError: undefined method `+' for nil:NilClass
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/path/pattern.rb:139:in `[]'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:127:in `block (2 levels) in find_routes'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `each'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `each_with_index'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `block in find_routes'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:123:in `map!'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:123:in `find_routes'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:32:in `serve'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/routing/route_set.rb:852:in `call'
```
While looking through various Rails issues related to the error above, I
came across https://github.com/rails/rails/pull/27647 which is a fix to
fully initialize routes before the first request is handled. However,
the routes are only fully initialize only if `config.eager_load` is set
to `true`. There is no reason why `config.eager_load` shouldn't be `true` in the
CI environment and this is what a new Rails 7.1 app is generated with.
What does this change do?
Enable `config.eager_load` when `env["CI"]` is present
Why this change?
`upload-artifacts` v4 introduces a breaking change where uploading
multiple uploads to the same name artifact is no longer supported.
However, we have external services reading these artifacts and the
change in the artifact name is breaking those services.
