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

Fix --nozip_undeclared_test_outputs on Windows #16973

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 9, 2022

Without this change, testing with --nozip_undeclared_test_outputs always fails with this error:

FATAL: MappedOutputFile(): CreateFileW() failed: (error: 3): The system cannot find the path specified.

@fmeum fmeum force-pushed the fix-nozip-undeclared-test-outputs-on-windows branch 5 times, most recently from 5b0af74 to fd6897b Compare December 9, 2022 16:24
Without this change, testing with `--nozip_undeclared_test_outputs`
always fails with this error:

```
FATAL: MappedOutputFile(): CreateFileW() failed: (error: 3): The system cannot find the path specified.
```
@fmeum fmeum force-pushed the fix-nozip-undeclared-test-outputs-on-windows branch from fd6897b to ccd8f48 Compare December 9, 2022 16:45
], )
self.AssertExitCode(exit_code, 0, stderr)
# FIXME: The Windows test runner does not delete the undeclared outputs
# after zipping, which differs from the behavior on other platforms.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing this would require implementing recursive directory removal in the Windows runner. The code already exists in src/main/cpp/util/file_windows.cc, let me know if you want me to copy it over.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with fixing this separately.

@fmeum
Copy link
Collaborator Author

fmeum commented Dec 9, 2022

@tjgq As the author of 87a30ce, could you review this bug fix?

@fmeum fmeum marked this pull request as ready for review December 9, 2022 17:00
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Dec 10, 2022
], )
self.AssertExitCode(exit_code, 0, stderr)
# FIXME: The Windows test runner does not delete the undeclared outputs
# after zipping, which differs from the behavior on other platforms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with fixing this separately.

@tjgq tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 12, 2022
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 12, 2022

@bazel-io flag

@fmeum
Copy link
Collaborator Author

fmeum commented Dec 12, 2022

(flagging for 6.1.0, not a regression)

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 12, 2022
@fmeum fmeum deleted the fix-nozip-undeclared-test-outputs-on-windows branch December 13, 2022 09:39
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Dec 13, 2022
@meteorcloudy
Copy link
Member

@bazel-io fork 6.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 14, 2022
ShreeM01 added a commit that referenced this pull request Jan 25, 2023
Without this change, testing with `--nozip_undeclared_test_outputs` always fails with this error:

```
FATAL: MappedOutputFile(): CreateFileW() failed: (error: 3): The system cannot find the path specified.
```

Closes #16973.

PiperOrigin-RevId: 494953454
Change-Id: I8338e897a9e4f4fdcd6dad511ee8e777182fefcd

Co-authored-by: Fabian Meumertzheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants