From 4e34f672fff23dc7659e2966c0a131d836f2f23e Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 17 Jan 2020 00:00:09 -0800 Subject: [PATCH] docs: Reorganize testing.md and using.md. This is a fairly involved set of changes, including changes that: * Delete various legacy or semi-duplicated sections of testing.md. Nobody needs to manually delete the postgres datbase anymore, as reflected in the fact that the docs still mention postgres 9.1 from Ubuntu Precise. * Simplify the distracting heading section at the top of testing.md. * Move content on manual testing to docs/development/using.md. * Moves some content related to managing the database to schema-migrations.md. (Resulting in some cleanups to that page as well). I ideally would have split this into smaller pieces. --- docs/development/using.md | 64 ++++++---- docs/subsystems/schema-migrations.md | 70 ++++++----- docs/testing/testing.md | 178 ++++++--------------------- 3 files changed, 122 insertions(+), 190 deletions(-) diff --git a/docs/development/using.md b/docs/development/using.md index 1d213edc11..c1629af1a4 100644 --- a/docs/development/using.md +++ b/docs/development/using.md @@ -11,15 +11,28 @@ If you're working on authentication methods or need to use the [Zulip REST API][rest-api], which requires an API key, see [authentication in the development environment][authentication-dev-server]. -## Common (linting and testing) +## Common +* Zulip's master branch moves quickly, and you should rebase + constantly with e.g. `git fetch upstream; git rebase + upstream/master` to avoid developing on an old version of the Zulip + codebase (leading to unnecessary merge conflicts). +* Remember to run `tools/provision` to update your development + environment after switching branches; it will run in under a second + if no changes are required. * After making changes, you'll often want to run the [linters](../testing/linters.md) and relevant [test - suites](../testing/testing.md). All of our test suites are designed - to support quickly testing just a single file or test case, which - you should take advantage of to save time. Consider using our [Git - pre-commit hook](../git/zulip-tools.html#set-up-git-repo-script) to - automatically lint changes when you commit them. + suites](../testing/testing.md). Consider using our [Git pre-commit + hook](../git/zulip-tools.html#set-up-git-repo-script) to + automatically lint whenever you make a commit. +* All of our test suites are designed to support quickly testing just + a single file or test case, which you should take advantage of to + save time. +* Many useful development tools, including tools for rebuilding the + database with different test data, are documented in-app at + `https://localhost:9991/devtools`. +* If you want to restore your development environment's database to a + pristine state, you can use `./tools/do-destroy-rebuild-database`. ## Server @@ -32,24 +45,23 @@ the development environment][authentication-dev-server]. they use. You can watch this happen in the `run-dev.py` console to make sure the backend has reloaded. * The Python queue workers will also automatically restart when you - save changes. However, you may need to ctrl-C and then restart - `run-dev.py` manually if a queue worker has crashed. + save changes, as long as they haven't crashed (which can happen if + they reloaded into a version with a syntax error). * If you change the database schema (`zerver/models.py`), you'll need to use the [Django migrations - process](../subsystems/schema-migrations.md) to create and then run - your migrations; see the [new feature - tutorial][new-feature-tutorial] for an example. + process](../subsystems/schema-migrations.md); see also the [new + feature tutorial][new-feature-tutorial] for an example. * While testing server changes, it's helpful to watch the `run-dev.py` console output, which will show tracebacks for any 500 errors your - Zulip development server encounters. -* To manually query the Postgres database interactively, use - `./manage.py shell` or `manage.py dbshell` depending whether you - prefer an iPython shell or SQL shell. + Zulip development server encounters (which are probably caused by + bugs in your code). +* To manually query Zulip's database interactively, use `./manage.py + shell` or `manage.py dbshell`. * The database(s) used for the automated tests are independent from the one you use for manual testing in the UI, so changes you make to the database manually will never affect the automated tests. -## Web +## Web application * Once the development server (`run-dev.py`) is running, you can visit in your browser. @@ -60,18 +72,22 @@ the development environment][authentication-dev-server]. something other than the login process. * You can test the login or registration process by clicking the links for the normal login page. -* If you change CSS files, your changes will appear immediately via - webpack hot module replacement. -* If you change JavaScript code (`static/js`) or Handlebars templates - (`static/templates`), the browser window will be reloaded - automatically. -* For Jinja2 backend templates (`templates/*`), you'll need to reload - the browser manually to see changes take effect. +* Most changes will take effect automatically. Details: + * If you change CSS files, your changes will appear immediately via + webpack hot module replacement. + * If you change JavaScript code (`static/js`) or Handlebars + templates (`static/templates`), the browser window will be + reloaded automatically. + * For Jinja2 backend templates (`templates/*`), you'll need to reload + the browser window to see your changes. * Any JavaScript exceptions encountered while using the webapp in a development environment will be displayed as a large notice, so you don't need to watch the JavaScript console for exceptions. +* Both Chrome and Firefox have great debuggers, inspectors, and + profilers in their built-in developer tools. +* `debug.js` has some occasionally useful JavaScript profiling code. -## Mobile +## Mobile apps See the mobile project's documentation on [using a development server for mobile development][mobile-dev-server]. diff --git a/docs/subsystems/schema-migrations.md b/docs/subsystems/schema-migrations.md index 44cda54ea3..14517c576b 100644 --- a/docs/subsystems/schema-migrations.md +++ b/docs/subsystems/schema-migrations.md @@ -9,30 +9,23 @@ This page documents some important issues related to writing schema migrations. * If your database migration is just to reflect new fields in - `models.py`, you'll want to just run `manage.py migrations` to - generate a migration file (remember to `git add` it). If the - `models.py` change is a single field or table being added, the - automatically generated names are nice andx clear, but if the - migration changes more than one thing, you'll get a name determined - by the date like `0089_auto_20170710_1353.py` and will want to - rename the migration file. - -* **Naming**: Please provide clear names for new database migrations - (e.g. `0072_realmauditlog_add_index_event_time.py`). Since in the - Django migrations system, the filename is the name for the - migration, this just means moving the migration file to have a - reasonable name. Note that `tools/test-migrations` will fail in - Travis CI if a migration has bad name of the form - `0089_auto_20170710_1353.py`, which are what Django generates - automatically for nontrivial database schema changes. - -* **Large tables**: For large tables like Message and UserMessage, you - want to take precautions when adding columns to the table, - performing data backfills, or building indexes. We have a - `zerver/lib/migrate.py` library to help with adding columns and - backfilling data. For building indexes on these tables, we should do - this using SQL with postgres's CONCURRENTLY keyword. - + `models.py`, you'll typically want to just: + * Rebase your branch before you start (this may save work later). + * Update the model class definitions in `zerver/models.py`. + * Run `./manage.py makemigrations` to generate a migration file + * Rename the migration file to have a descriptive name if Django + generated used a date-based name like `0089_auto_20170710_1353.py` + (which happens when the changes are to multiple models and Django). + * `git add` the new migration file + * Run `tools/provision` to update your local database to apply the + migrations. + * Commit your changes. +* For more complicated migrations where you need to run custom Python + code as part of the migration, it's best to read past migrations to + understand how to write them well. `git grep RunPython + zerver/migrations/02*` will find many good examples. Before writing + migrations of this form, you should read Django's docs and the + sections below. * **Numbering conflicts across branches**: If you've done your schema change in a branch, and meanwhile another schema change has taken place, Django will now have two migrations with the same @@ -50,12 +43,19 @@ migrations. you automatically (you still need to do all the `git add` commands, etc.). +* **Large tables**: For our very largest tables (e.g. Message and + UserMessage), we often need to take precautions when adding columns + to the table, performing data backfills, or building indexes. We + have a `zerver/lib/migrate.py` library to help with adding columns + and backfilling data. For building indexes on these tables, we + should do this using SQL with postgres's CONCURRENTLY keyword. + * **Atomicity**. By default, each Django migration is run atomically inside a transaction. This can be problematic if one wants to do something in a migration that touches a lot of data and would best be done in batches of e.g. 1000 objects (e.g. a `Message` or - `UserMessage` table change). There is a new Django feature added in - [Django 1.10][migrations-non-atomic] that makes it possible to add + `UserMessage` table change). There is a [useful Django + feature][migrations-non-atomic] that makes it possible to add `atomic=False` at the top of a `Migration` class and thus not have the entire migration in a transaction. This should make it possible to use the batch update tools in `zerver/lib/migrate.py` (originally @@ -83,8 +83,8 @@ migrations. situation you should use Django's `apps.get_model` to get access to a model as it is at the time of a migration. Note that this will work for doing something like `Realm.objects.filter(..)`, but - shouldn't be used for accessing `Realm.subdomain` or anything not - related to the Django ORM. + shouldn't be used for accessing properties like `Realm.subdomain` or + anything not related to the Django ORM. * **Making large migrations work**. Major migrations should have a few properties: @@ -141,3 +141,17 @@ to undo without going to backups. [django-migration-test-blog-post]: https://www.caktusgroup.com/blog/2016/02/02/writing-unit-tests-django-migrations/ [migrations-non-atomic]: https://docs.djangoproject.com/en/1.10/howto/writing-migrations/#non-atomic-migrations + +## Schema and initial data changes + +If you follow the processes described above, `tools/provision` and +`tools/test-backend` should detect any changes to the declared +migrations and run migrations on (`./manage.py migrate`) or rebuild +the relevant database automatially as appropriate. + +Developing migrations can result in manual fiddling that leads to a +broken database state, however. For those situations, we have +`tools/do-destroy-rebuild-test-database` and +`tools/do-destroy-rebuild-database` available to rebuild the databases +used for `test-backend` and [manual testing](../development/using.md), +respectively. diff --git a/docs/testing/testing.md b/docs/testing/testing.md index ac5bfd4a43..8564b0574f 100644 --- a/docs/testing/testing.md +++ b/docs/testing/testing.md @@ -1,62 +1,63 @@ # Testing and writing tests -## Overview +Zulip takes pride in its extensive, carefully designed test suites. +For example, `test-backend` runs a complete test suite (~98% test +coverage; 100% on core code) for the Zulip server in under a minute on +a fast laptop; very few webapps of similar scope can say something +similar. -Zulip has a full test suite that includes many components. The most -important components are documented in depth in their own sections: +This page focused on the mechanics of running automated tests in a +[development environment](../development/overview.md); you may also +want to read about our [testing philosophy](../testing/philosophy.md) +and [continuous integration +setup](../testing/continuous-integration.md). -- [Django](../testing/testing-with-django.md): backend Python tests -- [Casper](../testing/testing-with-casper.md): end-to-end UI tests -- [Node](../testing/testing-with-node.md): unit tests for JS front end code -- [Linters](../testing/linters.md): Our parallel linter suite -- [CI details](continuous-integration.md): How all of these run in CI -- [Other test suites](#other-test-suites): Our various smaller test suites. - -This document covers more general testing issues, such as how to run the -entire test suite, how to troubleshoot database issues, how to manually -test the front end, etc. - -We also document [how to manually test the app](manual-testing.md). +Manual testing with a web browser is primarily discussed in the docs +on [using the development environment](../development/using.md). ## Running tests Zulip tests must be run inside a Zulip development environment; if -you're using Vagrant, you will need to enter the Vagrant environment -before running the tests: +you're using Vagrant, you may need to enter it with `vagrant ssh`. -``` -vagrant ssh -cd /srv/zulip -``` +You can run all of the test suites (similar to our continuous integration) +as follows: -Then, to run the full Zulip test suite, do this: ``` ./tools/test-all ``` -This runs the linter (`tools/lint`) plus all of our test suites; -they can all be run separately (just read `tools/test-all` to see -them). You can also run individual tests which can save you a lot of -time debugging a test failure, e.g.: +However, you will rarely want to do this while actively developing, +because it takes a long time. Instead, your edit/refresh cycle will +typically involve running subsets of the tests with commands like these: ``` -./tools/lint # Runs all the linters in parallel +./tools/lint zerver/lib/actions.py # Lint the file you just changed ./tools/test-backend zerver.tests.test_bugdown.BugdownTest.test_inline_youtube ./tools/test-backend BugdownTest # Run `test-backend --help` for more options ./tools/test-js-with-casper 09-navigation.js ./tools/test-js-with-node utils.js ``` -The above setup instructions include the first-time setup of test -databases, but you may need to rebuild the test database occasionally -if you're working on new database migrations. To do this, run: -``` -./tools/do-destroy-rebuild-test-database -``` +The commands above will all run in just a few seconds. Many more +useful options are discussed in each tool's documentation (e.g. +`./tools/test-backend --help`). + +## Major test suites + +Zulip has a handful of major tests suite that every developer will +eventually work with, each with its own page detailing how it works: + +- [Linters](../testing/linters.md): Our dozen or so linters run in parallel. +- [Django](../testing/testing-with-django.md): Server/backend Python tests. +- [Node](../testing/testing-with-node.md): JavaScript tests for the + frontend run via node.js. +- [Casper (deprecated)](../testing/testing-with-casper.md): End-to-end + UI tests run via a browser. ## Other test suites -Zulip also has about a dozen smaller tests suites: +Additionally, Zulip also has about a dozen smaller tests suites: - `tools/test-migrations`: Checks whether the `zerver/migrations` migration content the models defined in `zerver/models.py`. See our @@ -100,40 +101,7 @@ things to the environment) why they are not part of the handful of major test suites like `test-backend`, but they all contribute something valuable to helping keep Zulip bug-free. -### Possible testing issues - -- When running the test suite, if you get an error involving Git that looks like this: - - ``` - gitlint| An error occurred while executing '/usr/bin/git rev-list --max-count=-1 upstream/master..HEAD': b"fatal: ambiguous argument 'upstream/master..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'" - ``` - - ... then you may need to connect the Zulip upstream repository with the following command: - - ``` - git remote add -f upstream https://github.com/zulip/zulip.git - ``` - -- When running casper tests (`./tools/test-js-with-casper`), if you -get an error like this: - -``` -Running node_modules/.bin/casperjs test /srv/zulip/frontend_tests/casper_tests/00-realm-creation.js -internal/child_process.js:289 - var err = this._handle.spawn(options); - ^ - -TypeError: Bad argument -``` -... it means that phantomjs is not installed. You can install it by running -the following commands. - -```bash -cd node_modules/phantomjs-prebuilt -node install.js -``` - -### Internet access inside test suites +## Internet access inside test suites As a policy matter, the Zulip test suites should never make outgoing HTTP or other network requests. This is important for 2 major @@ -185,73 +153,7 @@ The one exception to this policy is our documentation tests, which will attempt to verify that the links included in our documentation aren't broken. Those tests end up failing nondeterministically fairly often, which is unfortunate, but there's simply no other correct way -to verify links other than attempting to access them. - -## Schema and initial data changes - -If you change the database schema or change the initial test data, you -have to regenerate the pristine test database by running -`tools/do-destroy-rebuild-test-database`. - -## Wiping the test databases - -You should first try running: `tools/do-destroy-rebuild-test-database` - -If that fails you should try to do: - - sudo -u postgres psql - > DROP DATABASE zulip_test; - > DROP DATABASE zulip_test_template; - -and then run `tools/do-destroy-rebuild-test-database` - -### Recreating the postgres cluster - -```eval_rst -.. warning:: - This is irreversible! Do it with care and never do this anywhere - in production. -``` - -If your postgres cluster (collection of databases) gets totally trashed -permissions-wise, and you can't otherwise repair it, you can recreate -it. On Ubuntu: - - sudo pg_dropcluster --stop 9.1 main - sudo pg_createcluster --locale=en_US.utf8 --start 9.1 main - -## Local browser testing (local app + web browser) - -This section is about troubleshooting your local development environment. - -There is a [separate manual testing doc](manual-testing.md) that -enumerates things you can test as part of manual QA. - -### Clearing the development database - -You can use: - - ./tools/do-destroy-rebuild-database - -to drop the database on your development environment and repopulate -your it with the Shakespeare characters and some test messages between -them. This is run automatically as part of the development -environment setup process, but is occasionally useful when you want to -return to a clean state for testing. - -### JavaScript manual testing - -`debug.js` has some tools for profiling JavaScript code, including: - -- \`print\_elapsed\_time\`: Wrap a function with it to print the time - that function takes to the JavaScript console. -- \`IterationProfiler\`: Profile part of looping constructs (like a - for loop or \$.each). You mark sections of the iteration body and - the IterationProfiler will sum the costs of those sections over all - iterations. - -Chrome has a very good debugger and inspector in its developer tools. -Firebug for Firefox is also pretty good. They both have profilers, but -Chrome's is a sampling profiler while Firebug's is an instrumenting -profiler. Using them both can be helpful because they provide different -information. +to verify links other than attempting to access them. The compromise +we've implemented is that in CI, these tests only verify links to +websites controlled by the Zulip project (zulipchat.com, our GitHub, +our ReadTheDocs), and not links to third-party websites.