-
Notifications
You must be signed in to change notification settings - Fork 19
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
Jsonnet config #52
Jsonnet config #52
Conversation
* apply read symstem env with optional dotenv for .yaml * export VALIDATOR1_MNEMONIC=$VALIDATOR1_MNEMONIC && VALIDATOR2_MNEMONIC=$VALIDATOR2_MNEMONIC COMMUNITY_MNEMONIC=$COMMUNITY_MNEMONIC SIGNER1_MNEMONIC=$SIGNER1_MNEMONIC SIGNER2_MNEMONIC=$SIGNER2_MNEMONIC CRONOS_ADMIN=$CRONOS_ADMIN IBC_CRO_DENOM=$IBC_CRO_DENOM && poetry run pystarport serve --data=./data --config=pystarport/tests/test_expansion/cronos_has_posix_no_dotenv(*.yaml, *.jsonnet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just think we don't need to change the existing yaml test cases, just add new ones.
And could you add a changelog.
cronos_has_dotenv = parent / "cronos_has_dotenv.yaml" | ||
cronos_no_dotenv = parent / "cronos_no_dotenv.yaml" | ||
cronos_has_posix_no_dotenv = parent / "cronos_has_posix_no_dotenv.yaml" | ||
for type in [".yaml", ".jsonnet"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: looks like a good case to use https://docs.pytest.org/en/6.2.x/parametrize.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need call function, cant use parameter ar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it'll run your test case once for each parameter automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems can pass function as parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Only one minor suggestion is use parametrized feature of pytest to structure the yaml/jsonnet test case.
No description provided.