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.

Sign up for GitHub

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

Jump to bottom

--test_env (and some other test flags) should not discard analysis cache with --trim_test_configuration #7450

Open
or-shachar opened this issue Feb 17, 2019 · 15 comments · May be fixed by #24398
Open

--test_env (and some other test flags) should not discard analysis cache with --trim_test_configuration #7450

or-shachar opened this issue Feb 17, 2019 · 15 comments · May be fixed by #24398
Labels
not stale Issues or PRs that are inactive but not considered stale P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug

Comments

@or-shachar
Copy link
Contributor

Description of the problem / feature request:

My .bazelrc file has some options linked to test

test --test_arg=--jvm_flags=-Dcom.google.testing.junit.runner.shouldInstallTestSecurityManager=false

Apparently this makes analysis cache to be discarded between build and test invocations.

INFO: Build option --test_arg has changed, discarding analysis cache.

To fix it I need to link the option to build, which doesn't make a lot of sense as it's a test related.

Feature requests: what underlying problem are you trying to solve with this feature?

--test_arg should not affect the analysis cache. Or the warning message should be better.
BTW: same goes for --test_env

What operating system are you running Bazel on?

OSX

What's the output of bazel info release?

release 0.22.0
@or-shachar
Copy link
Contributor Author

or-shachar commented Feb 17, 2019 •
edited
Loading

FYI @sha1n @ittaiz

@jin jin added team-Configurability platforms, toolchains, cquery, select(), config transitions untriaged labels Feb 17, 2019
@sha1n
Copy link

sha1n commented Feb 18, 2019

Same behaviour is observed on Ubuntu + Bazel version 0.21
@or-shachar @ittaiz

@ittaiz
Copy link
Member

ittaiz commented Feb 18, 2019

Can you try with test trim true?

@sha1n
Copy link

sha1n commented Feb 19, 2019

Thanks @ittaiz! Adding that flag indeed has an effect, but only on test_args. So, the presence of test_env causes the analysis cache to be discarded, with and without that flag.

@programmablereya programmablereya changed the title --test_arg should not discard analysis cache --test_env (and some other test flags) should not discard analysis cache with --trim_test_configuration Feb 19, 2019
@programmablereya
Copy link

Unfortunately, --trim_test_configuration not working with --test_env is known (as mentioned in issue #5579). :( This is a good canonical issue for the issue of resolving that discrepancy, however! :)

@programmablereya programmablereya added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Feb 19, 2019
@sha1n
Copy link

sha1n commented Feb 20, 2019

Thanks @serynth!
Can you elaborate more about why it doesn't work and whether or not there are plans to fix it, assuming that it is not the desired behaviour. Also, if there's any issue we can use to track progress around that area, it will be nice.

@programmablereya
Copy link

Sure! :) It definitely isn't the desired behavior, though whether it's fixed by --trim_test_configuration being fixed or when automatic trimming comes to fruition depends some on my schedule.

Why it doesn't work:
The flags named in #5579 (comment) are not in the TestConfiguration fragment, which is what --trim_test_configuration trims. Moving them requires doing some work to extract them from BuildConfiguration.Options and to remove BuildConfiguration's direct dependency on them. This is something I'd like to do, but automatic trimming is consuming my attention right now.

As I mentioned, this is (now) the canonical issue for tracking progress on this work.

@sha1n
Copy link

sha1n commented Feb 25, 2019

Thank you @serynth! 👍

@aiuto
Copy link
Contributor

aiuto commented Jun 8, 2020

@sdtwigg Can you look at this while you are finishing up on --trim_test_configuration?

@ulfjack
Copy link
Contributor

ulfjack commented Jun 9, 2020

--test_env currently lives in CoreOptions and is copied into BuildConfiguration, which invalidates the analysis phase. It should be straightforward to move it to TestConfiguration. It's also accessed in CommandEnvironment (Yuk!) for use in the ClientEnv bazel info key (but that shouldn't be a blocker AFAICT).

Changes here also need to take into account Google's fork of the test strategies, which may prevent external contributions.

@sdtwigg
Copy link
Contributor

sdtwigg commented Jun 9, 2020

Acknowledging this.

There is at least one minor hurdle. Internally, a small amount of rules have config_settings that are sensitive to --test_env, which requires special consideration before test_env can move under TestConfiguration.

@sdtwigg sdtwigg mentioned this issue Nov 1, 2021
Open
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 16, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@sgowroji sgowroji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
@aiuto
Copy link
Contributor

aiuto commented Feb 16, 2023 via email

@sgowroji sgowroji reopened this Feb 16, 2023
@sgowroji sgowroji added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Feb 16, 2023
@gregestren
Copy link
Contributor

No.

@fmeum fmeum mentioned this issue Nov 13, 2024
Closed
@fmeum fmeum linked a pull request Nov 19, 2024 that will close this issue
Open
@gregestren gregestren mentioned this issue Dec 5, 2024
Open
@fmeum fmeum reopened this Dec 5, 2024
@fmeum
Copy link
Collaborator

fmeum commented Dec 5, 2024

Reopening temporarily as the fixing commit has been rolled back. A roll forward is in review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
not stale Issues or PRs that are inactive but not considered stale P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug
Projects
None yet
Milestone
No milestone