Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(seed): run rebuilds in parallel, add perf logs #4334

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5bddaac
perf(seed): run rebuilds in parallel, add perf logs
ThatOneBro Apr 8, 2024
e0f710a
cleanup: remove stray performance.mark
ThatOneBro Apr 8, 2024
f3941d3
feat(server): add both `serial` and `parallel` modes for seed
ThatOneBro Apr 9, 2024
1c424bc
test(seed): split out seed tests, test in parallel to main tests
ThatOneBro Apr 23, 2024
ba9eaa8
cleanup(compose): rm stray service
ThatOneBro Apr 23, 2024
f98f105
cleanup(package.json): rm stray package
ThatOneBro Apr 23, 2024
6ca8cd1
cleanup(tsconfig): add `seed-tests` dir to tsconfig
ThatOneBro Apr 23, 2024
340ee80
fix(test.sh): make parallel tests fail together
ThatOneBro Apr 23, 2024
e5a9888
fix(actions/build): fix `postgres_seed` port
ThatOneBro Apr 23, 2024
fb90a1a
test(actions/build): see if hardcoding port works
ThatOneBro Apr 23, 2024
b4dfc20
cleanup(coverage): `coverage/seed/{parallel,serial}`
ThatOneBro Apr 23, 2024
ec0746b
cleanup(server/test): don't call `docker-compose` in `test` cmd
ThatOneBro Apr 23, 2024
10bc983
test(coverage): add back coverage of `src` to `seed` tests
ThatOneBro Apr 23, 2024
472be29
test(coverage): add exclusions too
ThatOneBro Apr 23, 2024
f99b615
fix(tests/seed): do minimal setup in seed tests to actually test seeding
ThatOneBro Apr 23, 2024
7e18ad8
cleanup(sonar): rm unnecessary `as`s, use `??` operator
ThatOneBro Apr 23, 2024
dfe2307
refactor(seed): always build promise arr first
ThatOneBro Apr 23, 2024
d71b8cd
refactor(seed): move seed tests back into `src`, filter them
ThatOneBro Apr 24, 2024
8369583
docs(server/jest): add comment about weird glob behavior
ThatOneBro Apr 24, 2024
5bdd757
docs(testing): note running `test:seed:parallel` on fresh install
ThatOneBro Apr 24, 2024
c13e908
revert(seed): restore separate rebuild paths
ThatOneBro Apr 24, 2024
743060f
test(seed): make serial seed test run migrations before test
ThatOneBro May 7, 2024
8080800
chore(package-lock): fix needless updates
ThatOneBro May 7, 2024
ed4a256
chore(package-lock): actually fix needless updates
ThatOneBro May 7, 2024
436298b
test(seed): stop needlessly busting cache
ThatOneBro May 8, 2024
8ad3add
test(seed): always bust cache
ThatOneBro May 8, 2024
8ef8b4b
tweak(jest): set the default test timeout lower
ThatOneBro May 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(tests/seed): do minimal setup in seed tests to actually test seeding
  • Loading branch information
ThatOneBro committed May 11, 2024
commit f99b6152cea91ebaec0ee258c9fca06af5b7033e
21 changes: 17 additions & 4 deletions packages/server/seed-tests/seed-serial.test.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,33 @@
import { Project } from '@medplum/fhirtypes';
import { initAppServices, shutdownApp } from '../src/app';
import { shutdownApp } from '../src/app';
import { loadTestConfig } from '../src/config';
import { getDatabasePool } from '../src/database';
import { AuthenticatedRequestContext, requestContextStore } from '../src/context';
import { getDatabasePool, initDatabase } from '../src/database';
import { SelectQuery } from '../src/fhir/sql';
import { loadStructureDefinitions } from '../src/fhir/structure';
import { initRedis } from '../src/redis';
import { seedDatabase } from '../src/seed';
import { withTestContext } from '../src/test.setup';

describe('Seed', () => {
describe('Seed Serial', () => {
beforeAll(async () => {
console.log = jest.fn();

const config = await loadTestConfig();
config.database.port = process.env['POSTGRES_SEED_PORT']
? Number.parseInt(process.env['POSTGRES_SEED_PORT'], 10)
: 5433;
return withTestContext(() => initAppServices(config));
// Keep Redis separate so caches between main test suite and this are separate
config.redis.db = 8;

// We load the minimal required to get things running so this actually tests seeding the database
return withTestContext(() =>
requestContextStore.run(AuthenticatedRequestContext.system(), async () => {
loadStructureDefinitions();
initRedis(config.redis);
await initDatabase(config);
})
);
});

afterAll(async () => {
Expand Down
17 changes: 14 additions & 3 deletions packages/server/seed-tests/seed.test.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move this file? Strong preference for keeping all .ts files in src/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was initially due to weird glob matching not excluding the seed tests properly from main line of tests... it was cleaner to just separate them since they are somewhat logically separate...

I put them back though, found a sort of hack around it with some ** paths that works for some reason. Made a note about it in the file

Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { Project } from '@medplum/fhirtypes';
import { initAppServices, shutdownApp } from '../src/app';
import { shutdownApp } from '../src/app';
import { loadTestConfig } from '../src/config';
import { getDatabasePool } from '../src/database';
import { AuthenticatedRequestContext, requestContextStore } from '../src/context';
import { getDatabasePool, initDatabase } from '../src/database';
import { SelectQuery } from '../src/fhir/sql';
import { loadStructureDefinitions } from '../src/fhir/structure';
import { initRedis } from '../src/redis';
import { seedDatabase } from '../src/seed';
import { withTestContext } from '../src/test.setup';

Expand All @@ -12,7 +15,15 @@ describe('Seed', () => {

const config = await loadTestConfig();
config.database.runMigrations = true;
return withTestContext(() => initAppServices(config));

// We load the minimal required to get things running so this actually tests seeding the database
return withTestContext(() =>
requestContextStore.run(AuthenticatedRequestContext.system(), async () => {
loadStructureDefinitions();
initRedis(config.redis);
await initDatabase(config);
})
);
});

afterAll(async () => {
Expand Down
2 changes: 0 additions & 2 deletions scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export NODE_OPTIONS='--max-old-space-size=8192'
rm -rf coverage
mkdir -p coverage/packages
mkdir -p coverage/combined
mkdir -p coverage/seed/serial
mkdir -p coverage/seed/parallel

npx concurrently -n seed,main --kill-others-on-fail "scripts/test-seed.sh" "scripts/test-main.sh"

Expand Down