Page MenuHomePhabricator

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

Authored by atul on May 23 2022, 2:15 PM.
Tags
None
Referenced Files
F3281720: D4110.id13074.diff
Sat, Nov 16, 9:58 AM
F3281683: D4110.id13078.diff
Sat, Nov 16, 9:52 AM
F3276413: D4110.id13024.diff
Sat, Nov 16, 7:28 AM
Unknown Object (File)
Thu, Nov 14, 4:38 PM
Unknown Object (File)
Thu, Nov 14, 10:23 AM
Unknown Object (File)
Thu, Nov 14, 10:23 AM
Unknown Object (File)
Thu, Nov 14, 10:23 AM
Unknown Object (File)
Thu, Nov 14, 10:23 AM

Details

Summary

NA

Test Plan

Will test via Buildkite CI

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

put "valid" json file in db_config

atul published this revision for review.May 23 2022, 2:27 PM
.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.

This revision is now accepted and ready to land.May 24 2022, 7:03 AM
This revision was automatically updated to reflect the committed changes.