Page MenuHomePhabricator

Refactor CLI tests to pass in contexts for managed and unmanaged GRPC servers
ClosedPublic

Authored by dgibson on Aug 7 2020, 9:17 PM.

Details

Summary

Apologies for length. This diff:

  • Splits the CLI tests per-command into different files (In hindsight should have done this first in a separate diff)
  • Adds contextmanagers that you can use to get CLI args for grpc servers (we need to spin up a server to get a correct --port / --socket argument), incorporates them into the params that we pass into each of the various tests.)
Test Plan

BK, Azure

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dgibson created this revision.Aug 7 2020, 9:17 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Aug 7 2020, 9:45 PM
Harbormaster failed remote builds in B16712: Diff 20384!
dgibson requested review of this revision.Aug 7 2020, 11:42 PM
dgibson edited the summary of this revision. (Show Details)Aug 7 2020, 11:58 PM
dgibson updated this revision to Diff 20409.Aug 10 2020, 4:30 PM

mock DagsterInstance.get() when calling CliRunner functions, so that run_launcher.join() will work

dgibson updated this revision to Diff 20427.Aug 11 2020, 3:51 PM

since this makes the test run notably slower, split it into 3 BK/Azure runs - one for CLIAPI, one for managed GPRC, and one for deployed GRPC

dgibson updated this revision to Diff 20428.Aug 11 2020, 3:58 PM

correct BK pipeline setup

dgibson updated this revision to Diff 20429.Aug 11 2020, 5:34 PM

combine the two grpc BK pipelines since they are faster than the CLI API pipeline

dgibson updated this revision to Diff 20430.Aug 11 2020, 5:44 PM

fix tox grpc

dgibson edited the summary of this revision. (Show Details)Aug 11 2020, 8:34 PM
This revision is now accepted and ready to land.Aug 13 2020, 9:39 PM
dgibson updated this revision to Diff 20544.Aug 14 2020, 12:00 AM
dgibson edited the summary of this revision. (Show Details)

up