refs 3c7a8dead4
- Simplifies the state initialization code significantly and reuses native boot mechanism instead of mimicking it (it was a headache to maintain with all the internal services moving around)
refs: TryGhost/Toolbox#147
* Replaces all references to isIgnitionError with isGhostError
* Switches use of GhostError to InternalServerError - as GhostError is no longer public
There are places where InternalServerError is not the valid error, and new errors should be added to the @tryghost/errors package to ensure that we can use semantically correct errors in those cases.
closes https://github.com/TryGhost/Toolbox/issues/148
- These regression tests introduce very little additional value in exchange for an expensive time to run them. Because we mostly care about stable support for the latest stable API version, the older API version test for "internal" API can go away
- In case there are bugs found we do care about in the v2/v3 APIs we can always revert some of these tests.
https://github.com/TryGhost/Toolbox/issues/140
- This test was bloating the regression/site suite and was using hacks (calling the Admin API) to create a custom redirects state
- It suits way better in e2e frontend test suite with less hacky approach to start the Ghsot instance - using custom routes file path to initialize the instance
refs https://github.com/TryGhost/Toolbox/issues/140
- Same/similar RSS tests were present in regression and e2e tests suites. It made sense to move missing cases from regression to e2e. This saves us time bootstraping db state in multiple places and keeps all test cases regarding single feature in same place
refs https://github.com/TryGhost/Toolbox/issues/139
- With ec2aed5ce8 the DynamiRedirectsManager has reached 100% test coverage and most of the tests present in the removed regression suite have been ported to unit tests
- No need to keep slow tests around! :)
refs https://github.com/TryGhost/Toolbox/issues/139
- The regression test suite for redirects functionality for way too big. And each restart was causing massive overhead. It's enough to have a single exhaustive test using multiple input files
- The tests testing API endpoints should've been e2e tests to start with
- The rest is covered in the unit tests for redirects api service
refs 91efa4605c
- Referenced commit introduced a double json-stringification to uploaded redirects.json files.
- The endpoint has no stability index of any sort and is meant to be dropped in Ghost v5. It's best to rework the redirects to the yaml format as descirbe here - https://ghost.org/docs/tutorials/implementing-redirects/#file-structure
refs https://github.com/TryGhost/Toolbox/issues/139
- The v2 and v3 redirects APIs are unofficial and should not be used by anyone in production. There's no good reason to maintain expensive to run test suites for old unofficial APIs.
- The test cases in canary suite covers the functionality of redirects enough to be sure they work as expeted
refs https://github.com/TryGhost/Toolbox/issues/138
- These tests still rely on the frontend to be present. Needs further investigation to remove "frontend: true" flag - it slows down test runs!
refs https://github.com/TryGhost/Toolbox/issues/138
- Final batch of the refactor to async/await syntax. Doing these refactors before modifying "testUtils.startGhost" everywhere to boot only with the backend
refs https://github.com/TryGhost/Toolbox/issues/138
- This is a continuation of a bigger refactor to use async/await syntax before migrating "startGhost" methods to only use backend boot
- Removed a little bit of dead code (like admin user creation) which should speed up test execution too!
- Refactored user variables to be declared closer to their usecases instead of being high up in a global scope - variables shoul not live that far apart from the code that uses them
refs https://github.com/TryGhost/Toolbox/issues/138
- First batch of the refactor to async/await syntax. Next one will cover the rest. Doing these refactors before modifying "testUtils.startGhost" everywhere to boot only with the backend
refs https://github.com/TryGhost/Toolbox/issues/138
- There is no good reason to keep this extra variable around just call "stop" in couple very specific cases. Even for those cases, there's `testUtils.stopGhost` method which achieves the same without additional variable to track.
refs https://github.com/TryGhost/Toolbox/issues/138
- Having the "ghost" alias only added cognitive load when reading through the test code and didn't provide any additional value. Removed the pattern to keep things simpler and more explicit
refs https://github.com/TryGhost/Toolbox/issues/138
- Using asycn/await syntax is way more readable and allows to identify further reusable patterns in test initialization. This refactor also served as an exploreation around how the code looks like at this point
refs https://github.com/TryGhost/Toolbox/issues/135
- Without sensible defaults the web app was not initializing either the backend nor the frontned parts of the application. Fixed the defaults so the problem doesn't happen again and optimized mock-express-style initialization to only initialize the frontend routing
refs: https://github.com/TryGhost/Toolbox/issues/130
The API version stays at v2 unless we stub the getFrontendApiVersion method. But stubbing the method doesn't get picked up unless we actually restart Ghost.
TODO: Maybe change the default here so we don't need to restart Ghost just to test the current version's API
https://github.com/TryGhost/Toolbox/issues/130
The transaction no longer commits in the promise chain, which wasn't
valid logic for a transaction, since it is commited automatically when
the promise chain resolves, and rollsback automatically when the
promise chain rejects.
This makes code which fails during the transaction error in the right
place, instead of getting stuck here. (Especially good for writing
tests).
The tests for this code can now live in the integration folder.
refs https://github.com/TryGhost/Team/issues/1212
This now emits the event when the service is reconfigured, rather than
when we issue the reconfigure command, which causes the event and the
action to be run in the wrong order. This would then cause knock on effects
of having the database in an undefined state - with stripe data in not linked
to the current Stripe account.
refs https://github.com/TryGhost/Team/issues/807
The launch wizard completed flag was previously stored at per user level in accessibility column of user table, so an administrator still got the option to complete the launch wizard even if the owner had completed it previously, which is not expected pattern. This change moves the launch complete flag for Admin to common settings from per user level so a site only needs to complete the launch wizard once irrespective of which user completes it
- adds new `editor_is_launch_complete` setting to track if a site launch steps are completed in Admin
- adds new migration util to easily allow adding new setting
- adds migration to introduce new `editor_is_launch_complete` setting
- adds migration to update launch complete flag for a site if any of the users have already completed the launch steps
refs refs https://linear.app/tryghost/issue/CORE-84/have-a-look-at-the-eggs-redirects-refactor-branch
- The tests needed to have a clean state with empty redirects file, which was previously ensured through "configUtils". Because configUtils don't play ball with the class initialization pattern this approach was chosen
- It's an end-to-end test with lots of logic and pobably would be enough to run against single API endpoint. Leaving it as is and to be improved in the future
refs https://linear.app/tryghost/issue/CORE-84/have-a-look-at-the-eggs-redirects-refactor-branch
- The problem this change is addressing is inability to override config values once the code is extracted into a class+DI pattern
- The work around is restarting the instance with the configuration testing expected behavior - in this case missing or existing types of redirects files
no issue
The way GA flags were introduced means that they stop existing in the `'labs'` setting in the db and are instead forced to always return `true` when checking the flag in the labs service. However, Admin which uses the flags fetches them via the `/settings/` API endpoint which was only returning the raw labs setting db value meaning GA flags appeared to be disabled unless the flag had previously been enabled and no settings save had occured.
- updated the settings bread service to replace the labs setting value with the JSON stringified output of `labs.getAll()` which is the ultimate source-of-truth for a feature being enabled/disabled
- extracted `browse()` behaviour to an internal `_formatBrowse()` method so we can apply the same filtering/modification for output of `browse()` and `edit()`
Co-authored-by: Fabien O'Carroll <fabien@allou.is>
closes https://github.com/TryGhost/Team/issues/1150
Our override of the base Bookshelf `insert` operation so that our own `formatOnWrite()` method is called on attributes was working on a false assumption that an `attrs` attribute is passed in as it is for the `update` operation. Instead Bookshelf's base update uses the `model.attributes` values to create an `attrs` object that is then passed through the usual `.format()` method meaning that our `insert` override was not actually doing anything.
- added a failing regression test for the `formatOnWrite()` override behaviour
- adjusted our insert/update overrides to set an internal `_isWriting` property on the model, then if that property is true our `.format()` override (which is called by Bookshelf on a generated `attrs` object during inserts) we manually call our `.formatOnWrite()` method
- updated both overrides even though `update` was working for consistency and less cognitive overhead for reasoning between two different approaches
- These don't make sense and we're working on improving testing across the board
- We'll make sure our testing best practices are documented when they've settled
- the integrationTesting utils are specific to the express mock style of testing
- all other tests can use the url-service-utils to check the url service is finished
- done a fastest-possible overhaul on this style of tests to try to get them to work independently again
This is a pattern that was introduced a while ago to try to speed up our e2e tests and I'm not sure if it's staying or going
It uses a minimal frontend-only version of the boot process and a custom-built express testing tool
However it's really old and out of date because of the boot refactor and several changes since
This highlights the key problem with it - it doesn't rely on any of our "core" boot process, it makes it up, and therefore how reliable are these tests?
Ideally we need to get these tests working with the real boot process in some capacity
We would then need to make sure we have all the tests in e2e-frontend written in this style
- this test file uses a different pattern to the other test files
- not yet sure if the pattern is terrible or genius, need to assess before moving it into a folder full of what are meant to be exemplary tests
closes https://github.com/TryGhost/Team/issues/1125
refs 3c822e0457
- Email-only is not considered a general availability feature and can be used without special flags.
- It allows to publish a new post type "email only" that only goes out as an email newletter and is available through an undescoverable URL (does not appear anywhere publicly similarly to preview posts) on the site.
- e2e tests are tests that cover critical functionality by booting ghost
- integration tests are more like unit tests, but need to initialise and use a db
- so settings shouldn't start Ghost, url service is critical and should be in integration, and preview is critical and should be in e2e
- some tests are necessarily driven from the db
- these are like unit tests, except they only make sense if using the db - else you have to stub too much to make them worthwhile
- for these rare but important cases, we have the clear concept of integration tests
- We have a bunch of important server-related e2e tests
- Make these clear in their own folder
- "server" is everything that isn't the api or the frontend - kind of a catch-all concept
refs https://github.com/TryGhost/Team/issues/1088
- adds schema for new offers table
- adds permission fixtures for new offers table
- adds migrations for new table and permissions
Co-authored-by: Fabien O'Carroll <fabien@allou.is>
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
refs 7528ec8c3b
- The way the custom redirects middleware was organized made it extremely hard to unit test it (had to stub the redirects service methods etc). With a new organization it's possible to provide needed redirects configs to the method which makes the actual redirects Router logic testable and the code less coupled with redirects services
- This was meant to be an attempt to extract more of the slow redirects regression tests, which failed. Instead found this weak spot that could be improved and gained:
- shaved 4s of time as two slow regression test cases are now gone
- there's now a base to build upon when getting more coverage for the custom redirects middleware
