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

Add environment variable support for Go tests #15963

Closed
Eric-Arellano opened this issue Jun 28, 2022 · 3 comments
Closed

Add environment variable support for Go tests #15963

Eric-Arellano opened this issue Jun 28, 2022 · 3 comments
Labels

Comments

@Eric-Arellano
Copy link
Contributor

With Python, we support setting env variables two ways:

  1. [test].extra_env_vars
  2. the extra_env_vars field

Neither is hooked up for Go, which is pure oversight.

Hook up [test].extra_env_vars

  1. Request TestExtraEnv in the @rule signature

@rule(desc="Test with Go", level=LogLevel.DEBUG)
async def run_go_tests(
field_set: GoTestFieldSet, test_subsystem: TestSubsystem, go_test_subsystem: GoTestSubsystem
) -> TestResult:

Like this:

@rule(level=LogLevel.DEBUG)
async def setup_pytest_for_target(
request: TestSetupRequest,
pytest: PyTest,
test_subsystem: TestSubsystem,
python_setup: PythonSetup,
coverage_config: CoverageConfig,
coverage_subsystem: CoverageSubsystem,
test_extra_env: TestExtraEnv,
global_options: GlobalOptions,
) -> TestSetup:

  1. Set the Process's env field to test_extra_env.env

https://github.com/pantsbuild/pants/blob/bf25e129288e80116220292786f7e6018184173e/src/python/pants/backend/go/goals/test.py#L311-L234

  1. Add a test to test_test.py, e.g.
    def test_extra_env_vars(rule_runner: RuleRunner) -> None:
    rule_runner.write_files(
    {
    f"{PACKAGE}/test_extra_env_vars.py": dedent(
    """\
    import os
    def test_args():
    assert os.getenv("ARG_WITH_VALUE_VAR") == "arg_with_value_var"
    assert os.getenv("ARG_WITHOUT_VALUE_VAR") == "arg_without_value_value"
    assert os.getenv("PYTHON_TESTS_VAR_WITH_VALUE") == "python_tests_var_with_value"
    assert os.getenv("PYTHON_TESTS_VAR_WITHOUT_VALUE") == "python_tests_var_without_value"
    assert os.getenv("PYTHON_TESTS_OVERRIDE_WITH_VALUE_VAR") == "python_tests_override_with_value_var_override"
    """
    ),
    f"{PACKAGE}/BUILD": dedent(
    """\
    python_tests(
    extra_env_vars=(
    "PYTHON_TESTS_VAR_WITHOUT_VALUE",
    "PYTHON_TESTS_VAR_WITH_VALUE=python_tests_var_with_value",
    "PYTHON_TESTS_OVERRIDE_WITH_VALUE_VAR=python_tests_override_with_value_var_override",
    )
    )
    """
    ),
    }
    )
    tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="test_extra_env_vars.py"))
    result = run_pytest(
    rule_runner,
    tgt,
    extra_args=[
    '--test-extra-env-vars=["ARG_WITH_VALUE_VAR=arg_with_value_var", "ARG_WITHOUT_VALUE_VAR", "PYTHON_TESTS_OVERRIDE_ARG_WITH_VALUE_VAR"]'
    ],
    env={
    "ARG_WITHOUT_VALUE_VAR": "arg_without_value_value",
    "PYTHON_TESTS_VAR_WITHOUT_VALUE": "python_tests_var_without_value",
    "PYTHON_TESTS_OVERRIDE_WITH_VALUE_VAR": "python_tests_override_with_value_var",
    },
    )
    assert result.exit_code == 0

Add field

  1. Register new field to go_package target, probably called test_extra_env_vars. See this for example:

class PythonTestsExtraEnvVarsField(StringSequenceField):
alias = "extra_env_vars"
help = softwrap(
"""
Additional environment variables to include in test processes.
Entries are strings in the form `ENV_VAR=value` to use explicitly; or just
`ENV_VAR` to copy the value of a variable in Pants's own environment.
This will be merged with and override values from [test].extra_env_vars.
"""
)

https://www.pantsbuild.org/docs/target-api for general docs on Target API

  1. Consume in the rule, like this:

field_set_extra_env_get = Get(
Environment, EnvironmentRequest(request.field_set.extra_env_vars.value or ())
)

extra_env = {
"PYTEST_ADDOPTS": " ".join(add_opts),
"PEX_EXTRA_SYS_PATH": ":".join(prepared_sources.source_roots),
**test_extra_env.env,
# NOTE: field_set_extra_env intentionally after `test_extra_env` to allow overriding within
# `python_tests`.
**field_set_extra_env,
}

(You have to update the FieldSet to have the field)

  1. Update the test from before so that you test the field too, including that the field overrides the option.
@wfscheper
Copy link
Contributor

wfscheper commented Jun 30, 2022

I just ran into this trying out pants with a personal project. Mind if I try my hand at a PR?

@Eric-Arellano
Copy link
Contributor Author

Please go for it, @wfscheper! Let me know if you have questions. If you're not yet on it, we're a friendly bunch on Slack :) https://www.pantsbuild.org/docs/getting-help

Eric-Arellano pushed a commit that referenced this issue Jul 5, 2022
This addresses #15963 by plumbing through `[test].extra_env_vars` and adding the `test_extra_env_vars` field to `go_package`.

[ci skip-rust]

[ci skip-build-wheels]
Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this issue Jul 5, 2022
This addresses pantsbuild#15963 by plumbing through `[test].extra_env_vars` and adding the `test_extra_env_vars` field to `go_package`.

[ci skip-rust]

[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this issue Jul 5, 2022
… (#16067)

Add support for extra env variables in go tests (#16013)

This addresses #15963 by plumbing through `[test].extra_env_vars` and adding the `test_extra_env_vars` field to `go_package`.

[ci skip-rust]

[ci skip-build-wheels]

Co-authored-by: Walter Scheper <ratlaw@gmail.com>
@thejcannon thejcannon added the backend: Go Go backend-related issues label Jul 12, 2022
alonsodomin added a commit that referenced this issue Aug 10, 2022
The implementation approach is based on the recommendations found in #15963 and, since this affects to several targets, similar as in #16387, where a base class for the field is created in core and then extended for each of the different target types.
cczona pushed a commit to cczona/pants that referenced this issue Sep 1, 2022
…16455)

The implementation approach is based on the recommendations found in pantsbuild#15963 and, since this affects to several targets, similar as in pantsbuild#16387, where a base class for the field is created in core and then extended for each of the different target types.
@tdyas
Copy link
Contributor

tdyas commented Dec 16, 2022

Closing since this was already implemented by #16013.

@tdyas tdyas closed this as completed Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants