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

Windows, test wrapper: --all_incompatible_changes should not yet activate new test wrapper #6859

Closed
laszlocsomor opened this issue Dec 7, 2018 · 3 comments
Closed

Windows, test wrapper: --all_incompatible_changes should not yet activate new test wrapper #6859

laszlocsomor opened this issue Dec 7, 2018 · 3 comments
Assignees
@laszlocsomor
Labels
area-Windows Windows-specific issues and feature requests P1 I'll work on this now. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug

Comments

@laszlocsomor
Copy link
Contributor

Description of the problem / feature request:

The Windows-native test wrapper (see #5508) is currently guarded by the --[no]incompatible_windows_native_test_wrapper flag, however it's not yet ready for dogfooding.
But since it's an "--incompatible..." flag, it's turned on by --all_incompatible_changes, which is supposedly used by some of the Bazel CI pipelines to test upcoming incompatible changes.
If the new test wrapper were turned on, it would certainly break tests on Windows, adding noise to those CI pipelines.
Therefore the flag should be renamed something else (maybe "--experimental...") until the feature is ready for dogfooding, after which it can be renamed to "--incompatible..." again.
@laszlocsomor laszlocsomor self-assigned this Dec 7, 2018
@laszlocsomor laszlocsomor added type: bug P0 This is an emergency and more important than other current work. (Assignee required) release blocker area-Windows Windows-specific issues and feature requests labels Dec 7, 2018
@laszlocsomor laszlocsomor mentioned this issue Dec 7, 2018
Closed
@laszlocsomor laszlocsomor added P1 I'll work on this now. (Assignee required) and removed release blocker P0 This is an emergency and more important than other current work. (Assignee required) labels Dec 7, 2018
@laszlocsomor
Copy link
Contributor Author

@laurentlb : I believe this is a release blocker for #6859, please confirm.

0.20.0 contains the flag as --[no]windows_native_test_wrapper.

0.21.0rc2 contains it as --[no]incompatible_windows_native_test_wrapper . If 0.21.0 is released with this flag, then anyone using the release with --all_incompatible_changes will activate this flag, breaking test execution on Windows. I think 0.21.0 should be released with this flag called "--experimental...".

@laszlocsomor laszlocsomor mentioned this issue Dec 7, 2018
Closed
@meteorcloudy
Copy link
Member

I think it's better to use --experimental_... if the feature is not yet completed. But Bazel CI doesn't use --all_incompatible_changes, we specify incompatible flags explicitly for now, so CI is not affected.

@laszlocsomor
Copy link
Contributor Author

That's great to know CI is not affected. Bazel users still might, though, so let's fix this bug.

laurentlb pushed a commit that referenced this issue Dec 14, 2018
@laszlocsomor @laurentlb
Windows, test wrapper: rename the associated flag
cdba8ce 
Rename:
--[no]incompatible_windows_native_test_wrapper
to:
--[no]experimental_windows_native_test_wrapper

in order to avoid triggering this flag with
--all_incompatible_changes.

The test wrapper is not yet ready for general use
so turning it on with this flag (e.g. on CI) is
undesirable.

See #5508
Fixes #6859

Closes #6858.

PiperOrigin-RevId: 224490751
laurentlb pushed a commit that referenced this issue Dec 17, 2018
@laszlocsomor @laurentlb
Windows, test wrapper: rename the associated flag
726e578 
Rename:
--[no]incompatible_windows_native_test_wrapper
to:
--[no]experimental_windows_native_test_wrapper

in order to avoid triggering this flag with
--all_incompatible_changes.

The test wrapper is not yet ready for general use
so turning it on with this flag (e.g. on CI) is
undesirable.

See #5508
Fixes #6859

Closes #6858.

PiperOrigin-RevId: 224490751
@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@laszlocsomor laszlocsomor

Labels
area-Windows Windows-specific issues and feature requests P1 I'll work on this now. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@philwo @meteorcloudy @laszlocsomor