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

testing.TestEnvironment not equal to itself #15225

Closed
rickeylev opened this issue Apr 12, 2022 · 4 comments
Closed

testing.TestEnvironment not equal to itself #15225

rickeylev opened this issue Apr 12, 2022 · 4 comments
Assignees
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: bug

Comments

@rickeylev
Copy link
Contributor

Description of the bug:

The testing.TestEnvironment object is a bit strange: it isn't equal to itself. In comparison, most provider keys are equal to themselves.

This makes it difficult to write code that can properly detect when the testing.TestEnvironment provider key was requested, e.g.

def get_provider(key):
  if key == testing.TestEnvironment: ...

(in my particular case, I'm writing helper code for tests to assert TestEnvironment)

Also, I'm not sure if this should be TestEnvironment or TestEnvironmentInfo -- the public symbol is testing.TestEnvironment, but the docs say it returns TestEnvironmentInfo (note no leading "testing" namespace and the extra Info suffix).

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

print(testing.TestEnvironment == testing.TestEnvironment)

actual: prints false

expected: prints true

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

google build

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

n/a

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

n/a

Have you found anything relevant by searching the web?

15224 is sort of related -- same category of "TestEnvironemt provider acts weird"

Any other information, logs, or outputs that you want to share?

No response

@brentleyjones
Copy link
Contributor

testing.TestEnvironment is the constructor, not the key, right? For example, testing.TestEnvironment in target doesn't work.

@brentleyjones
Copy link
Contributor

brentleyjones commented Apr 12, 2022

Related, I would love a way to query for the TestEnvironmentInfo in a Target. This TODO seems important:

// TODO(bazel-team): Change this function to be the actual TestEnvironmentInfo.PROVIDER.

Edit: And I see you filed that already: #15224 ❤️

@fmeum
Copy link
Collaborator

fmeum commented Apr 13, 2022

I'm working on refactoring TestEnvironmentInfo into a proper exported RunEnvironmentInfo provider (name is TBD) that can be used for any executable target, not just tests. That should fix #15224, #15225 and #7364.

@brandjon
Copy link
Member

It seems surprising to me that it wouldn't compare equal to itself, unless a new method descriptor is being created each time the method is retrieved. But I thought we cached those.

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Apr 28, 2022
fmeum added a commit to fmeum/bazel that referenced this issue Jun 29, 2022
The new RunEnvironmentInfo provider allows any executable or test
Starlark rule to specify the environment for when it is executed, either
as part of a test action or via the run command.

Refactors testing.TestEnvironment to construct a RunEnvironmentInfo and
adds a warning (but not an error) if the provider constructed in this
way is returned from a non-executable non-test rule. If a
RunEnvironmentInfo is constructed directly via the Starlark constructor,
this warning becomes an error.

Fixes bazelbuild#7364
Fixes bazelbuild#15224
Fixes bazelbuild#15225

Closes bazelbuild#15232.

PiperOrigin-RevId: 448185352
ckolli5 pushed a commit that referenced this issue Jun 29, 2022
* Specify fixedEnv/inheritedEnv interaction in ActionEnvironment

Previously, ActionEnvironment did not publicly document how fixed and
inherited environment variables interact, but still cautioned users to
keep the two sets disjoint without enforcing this. As a result, neither
could users rely on the interaction nor could ActionEnvironment benefit
from the additional freedom of not specifying the behavior.

With this commit, ActionEnvironment explicitly specifies that the values
of environment variable inherited from the client environment take
precedence over fixed values and codifies this behavior in a test.
This has been the effective behavior all along and has the advantage
that users can provide overrideable defaults for environment variables.

Closes #15170.

PiperOrigin-RevId: 439315634

* Intern trivial ActionEnvironment and EnvironmentVariables instances

When an ActionEnvironment is constructed out of an existing one, the
ActionEnvironment and EnvironmentVariables instances, which are
immutable, can be reused if no variables are added.

Also renames addVariables and addFixedVariables to better reflect that
they are operating on an immutable type.

Closes #15171.

PiperOrigin-RevId: 440312159

* Let Starlark executable rules specify their environment

The new RunEnvironmentInfo provider allows any executable or test
Starlark rule to specify the environment for when it is executed, either
as part of a test action or via the run command.

Refactors testing.TestEnvironment to construct a RunEnvironmentInfo and
adds a warning (but not an error) if the provider constructed in this
way is returned from a non-executable non-test rule. If a
RunEnvironmentInfo is constructed directly via the Starlark constructor,
this warning becomes an error.

Fixes #7364
Fixes #15224
Fixes #15225

Closes #15232.

PiperOrigin-RevId: 448185352

* Fix strict deps violation

```
src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java:990: error: [strict] Using type com.google.devtools.build.lib.cmdline.LabelValidator from an indirect dependency (TOOL_INFO: "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator"). See command below **
      LabelValidator.parseAbsoluteLabel(labelString);
      ^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants