Details

Reviewers

Commits

rCOMM2f3a72ed206f: [CI] Add `keyserver: yarn test` to CI

Summary

Test Plan

Will test via Buildkite CI

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Harbormaster returned this revision to the author for changes because remote builds failed.May 23 2022, 2:19 PM

put "valid" json file in db_config

atul published this revision for review.May 23 2022, 2:27 PM

> instead of >>

.buildkite/eslint_flow_jest.yml
6 ↗	(On Diff #13025)	Not great, but the `keyserver` tests fail if `db_config` isn't a valid JSON file. Specifically on `database.js:8`: import dbConfig from '../../secrets/db_config';

tomek added inline comments.

.buildkite/eslint_flow_jest.yml
6 ↗	(On Diff #13025)	I don't think we need to `touch` if we're writing to the `db_config`

This revision is now accepted and ready to land.May 24 2022, 2:20 AM

ashoat requested changes to this revision.May 24 2022, 6:31 AM

ashoat added inline comments.

.buildkite/eslint_flow_jest.yml
6 ↗	(On Diff #13025)	Do we still need this after D4097? I suspect we don't... the `import dbConfig` line is gone, and while some operations need the DB config to actually be specified, I assume all of those operations would fail with `{}` anyways.

This revision now requires changes to proceed.May 24 2022, 6:31 AM

.buildkite/eslint_flow_jest.yml
6 ↗	(On Diff #13025)	Additionally, if the `touch` commands are just here for Flow, I think we can get rid of them. I created ENG-1190 to track

retrigger without echo "{}" > db_config.json

add keyserver:yarn test to github actions workflow as well

atul marked 3 inline comments as done.May 24 2022, 6:46 AM

atul added inline comments.

.buildkite/eslint_flow_jest.yml
6 ↗	(On Diff #13025)	Do we still need this after D4097? I suspect we don't... the `import dbConfig` line is gone, and while some operations need the DB config to actually be specified, I assume all of those operations would fail with `{}` anyways. Nope, looks like we don't. Removed the `touch` command and updated this diff and CI continued to pass. Additionally, if the `touch` commands are just here for Flow, I think we can get rid of them. I created ENG-1190 to track Put up D4114 to address this.