This fixes a similar issue to 8b3eca0 where an Errno::ETXTBSY error was raised because the minio_runner gem was trying to install the binary across multiple processes in rspec. If we just make sure the latest version is installed before the tests run, this shouldn't happen, since MinioRunner.start will not do any further attempts at installation if the latest version is installed.
Embroider has been the default since b72ed3cb38d58d4cf316f39d7e9de0e722729b5b. This commit removes the ability to set `USE_EMBROIDER=0` and go back to the classic build.
This workflow runs only for code underneath the `migrations/` directory. The usual test workflow is skipped for migrations because running frontend and backend tests is a waste of time and resources when only migrations are changed.
Discourse core now builds and runs with Embroider! This commit adds
the Embroider-based build pipeline (`USE_EMBROIDER=1`) and start
testing it on CI.
The new pipeline uses Embroider's compat mode + webpack bundler to
build discourse code, and leave everything else (admin, wizard,
markdown-it, plugins, etc) exactly the same using the existing
Broccoli-based build as external bundles (<script> tags), passed
to the build as `extraPublicTress` (which just means they get
placed in the `/public` folder).
At runtime, these "external" bundles are glued back together with
`loader.js`. Specifically, the external bundles are compiled as
AMD modules (just as they were before) and registered with the
global `loader.js` instance. They expect their `import`s (outside
of whatever is included in the bundle) to be already available in
the `loader.js` runtime registry.
In the classic build, _every_ module gets compiled into AMD and
gets added to the `loader.js` runtime registry. In Embroider,
the goal is to do this as little as possible, to give the bundler
more flexibility to optimize modules, or omit them entirely if it
is confident that the module is unused (i.e. tree-shaking).
Even in the most compatible mode, there are cases where Embroider
is confident enough to omit modules in the runtime `loader.js`
registry (notably, "auto-imported" non-addon NPM packages). So we
have to be mindful of that an manage those dependencies ourselves,
as seen in #22703.
In the longer term, we will look into using modern features (such
as `import()`) to express these inter-dependencies.
This will only be behind a flag for a short period of time while we
perform some final testing. Within the next few weeks, we intend
to enable by default and remove the flag.
---------
Co-authored-by: David Taylor <david@taylorhq.com>
This commit adds some system specs to test uploads with
direct to S3 single and multipart uploads via uppy. This
is done with minio as a local S3 replacement. We are doing
this to catch regressions when uppy dependencies need to
be upgraded or we change uppy upload code, since before
this there was no way to know outside manual testing whether
these changes would cause regressions.
Minio's server lifecycle and the installed binaries are managed
by the https://github.com/discourse/minio_runner gem, though the
binaries are already installed on the discourse_test image we run
GitHub CI from.
These tests will only run in CI unless you specifically use the
CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars.
For a history of experimentation here see https://github.com/discourse/discourse/pull/22381
Related PRs:
* https://github.com/discourse/minio_runner/pull/1
* https://github.com/discourse/minio_runner/pull/2
* https://github.com/discourse/minio_runner/pull/3
We can no long user Webdriver - SeleniumHQ/selenium#11066. Bumping selenium-webdriver did the trick, as well as manually setting the user_agent for mobile system specs. Unsure what changed to make this necessary, but it is necessary to get the app to boot in mobile view.
Why this change?
This is abit of a trial and error but we're starting to see selenium
session not created errors on CI. One of the reason for this is that the
system has run out of resources to create a new tab.
This commit reduces the number of parallel test processors in an attempt
to increase the amount of resources available to each test process and
hopefully lead to more stable CI system tests.
This reverts commit 865f7a98529f2190478308d34597b44e4bf5edba.
The flakiness that we have been seeing and fixing on CI were not related
to system resource problems. Therefore, we can bump this up back to 5.
Why is this change required?
We've been seeing flaky tests due to server errors on CI but are unable
to debug it because we do not log any of the errors. This change gives
us a fighting chance the next time we encounter a server error during
system test runs.
See
https://github.com/discourse/discourse/actions/runs/5459248864/jobs/9935049920?pr=22424
for an example of server errors encountered during system tests.
Using the runtime information, we will be able to more efficiently group
the test files across the test processes hence leading to better
utilization of resources.
Using the runtime information, we will be able to more efficiently group
the test files across the test processes hence leading to better
utilization of resources.
4dd053a69c073e7662f1718f70160b67deb4fd49 addressed most of the
instability we were seeing with system tests on CI and locally. Let's
try pushing the number of parallel processes up to squeeze as much time
savings as possible from the runner.
We were not setting capybara's default max wait time correctly in CI due
to a spelling error.
This regressd in fc17045876bc31f8bf8018f10b1f90e6736b461b
We're running on pretty crappy hardware on Github's CI and this has an
impact on the stability of our system tests on CI. Therefore, we are
bumping `CABPYARA_DEFAULT_MAX_WAIT_TIME` to 10 seconds to account for
the less than ideal hardware we're running the system tests on.
This change trades off speed for stability but speed is already bad on
CI so stability is more important for our case.
## How does this work?
Any time a lint rule is added or changed, you can run `yarn lint:fix` to handle all the auto-fixable situations.
But not all lints are auto-fixable -- for those, lint-to-the-future has tooling to automatically ignore present violations.
An alias has been added for lint-to-the-future to ignore new violations, `yarn lttf:ignore`.
The command will add lint-ignore declarations throughout all the files with present violations, which should then be committed.
An excerpt from lint-to-the-future's [README](https://github.com/mansona/lint-to-the-future#lint-to-the-future-dashboard):
> The point of Lint to the Future is to allow you to progressively update your codebase using new lint rules without overwhelming you with the task. You can easily ignore lint rules using project-based ignores in your config files but that doesn't prevent you from making the same errors in new files.
> We chose to do the ignores on a file basis as it is a perfect balance and it means that the tracking/graphing aspects of Lint to the Future provide you with achievable goals, especially in large codebases.
## How do I view progress?
lint-to-the-future provides graphs of violations-over-time per lint rule in a dashboard format, so we can track how well we're doing at cleaning up the violations.
To view the dashboard locally, run `yarn lint-progress` and visit `http://localhost:8084` (or whatever the port it chose, as it will choose a new port if 8084 is preoccupied)
Also there is a `list` command which shows a JSON object of:
```ts
{
[date: string]: { // yyyy-mm-dd
[pluginName: string]: {
[fileName: string]: string[]; // list of files with violations
}
}
}
```
```bash
yarn lint-to-the-future list --stdout
```
## What about lint-todo?
Lint todo is another system available for both eslint and ember-template-lint that _forces_ folks to "leave things better than they found them" by being transparent / line-specific ignoring of violations.
It was decided that for _this_ project, it made more sense, and would be less disruptive to new contributors to have the ignore declarations explicitly defined in each file (whereas in lint-todo, they are hidden).
To effectively use lint-todo, a whole team needs to agree to the workflow, and in open source, we want "just anyway" to be able to contribute, and throwing surprises at them can deter contributions.
In production, `eager_load=true`. This sometimes leads to boot errors which are not present in dev/test environments. Running `zeitwerk:check` in CI will help us to pick up on any errors early.
This commit also introduces a `DISCOURSE_ZEITWERK_EAGER_LOAD` environment variable to make it easier to toggle the behaviour when developing locally.
