-
-
Notifications
You must be signed in to change notification settings - Fork 636
Fix windows acceptance tests #641
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,6 +98,7 @@ filegroup( | |
name = "files", | ||
srcs = glob( | ||
include = [ | ||
"*.exe", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The interpreter was missing from |
||
"bin/**", | ||
"DLLs/**", | ||
"extensions/**", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,12 @@ | |
|
||
load("//python:versions.bzl", "PLATFORMS", "TOOL_VERSIONS") | ||
|
||
_WINDOWS_RUNNER_TEMPLATE = """\ | ||
@ECHO OFF | ||
set PATHEXT=.COM;.EXE;.BAT | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way windows figures out something is executable is through this environment variable. |
||
powershell.exe -c "& ./{interpreter_path} {run_acceptance_test_py}" | ||
""" | ||
|
||
def _acceptance_test_impl(ctx): | ||
workspace = ctx.actions.declare_file("/".join([ctx.attr.python_version, "WORKSPACE"])) | ||
ctx.actions.expand_template( | ||
|
@@ -52,7 +58,8 @@ def _acceptance_test_impl(ctx): | |
}, | ||
) | ||
|
||
py3_runtime = ctx.toolchains["@bazel_tools//tools/python:toolchain_type"].py3_runtime | ||
toolchain = ctx.toolchains["@bazel_tools//tools/python:toolchain_type"] | ||
py3_runtime = toolchain.py3_runtime | ||
interpreter_path = py3_runtime.interpreter_path | ||
if not interpreter_path: | ||
interpreter_path = py3_runtime.interpreter.short_path | ||
|
@@ -61,9 +68,9 @@ def _acceptance_test_impl(ctx): | |
executable = ctx.actions.declare_file("run_test_{}.bat".format(ctx.attr.python_version)) | ||
ctx.actions.write( | ||
output = executable, | ||
content = "call {interpreter_path} {run_acceptance_test_py}".format( | ||
interpreter_path = interpreter_path.replace("/", "\\"), | ||
run_acceptance_test_py = run_acceptance_test_py.short_path.replace("/", "\\"), | ||
content = _WINDOWS_RUNNER_TEMPLATE.format( | ||
interpreter_path = interpreter_path.replace("../", "external/"), | ||
run_acceptance_test_py = run_acceptance_test_py.short_path, | ||
), | ||
is_executable = True, | ||
) | ||
|
@@ -96,26 +103,40 @@ def _acceptance_test_impl(ctx): | |
)] | ||
|
||
_acceptance_test = rule( | ||
_acceptance_test_impl, | ||
implementation = _acceptance_test_impl, | ||
doc = "TODO", | ||
attrs = { | ||
"is_windows": attr.bool(mandatory = True), | ||
"python_version": attr.string(mandatory = True), | ||
"test_location": attr.string(mandatory = True), | ||
"is_windows": attr.bool( | ||
doc = "TODO", | ||
mandatory = True, | ||
), | ||
"python_version": attr.string( | ||
doc = "TODO", | ||
mandatory = True, | ||
), | ||
"test_location": attr.string( | ||
doc = "TODO", | ||
mandatory = True, | ||
), | ||
"_build_bazel_tmpl": attr.label( | ||
doc = "TODO", | ||
allow_single_file = True, | ||
default = "//python/tests/toolchains/workspace_template:BUILD.bazel.tmpl", | ||
default = Label("//python/tests/toolchains/workspace_template:BUILD.bazel.tmpl"), | ||
), | ||
"_python_version_test": attr.label( | ||
doc = "TODO", | ||
allow_single_file = True, | ||
default = "//python/tests/toolchains/workspace_template:python_version_test.py", | ||
default = Label("//python/tests/toolchains/workspace_template:python_version_test.py"), | ||
), | ||
"_run_acceptance_test": attr.label( | ||
doc = "TODO", | ||
allow_single_file = True, | ||
default = "//python/tests/toolchains:run_acceptance_test.py", | ||
default = Label("//python/tests/toolchains:run_acceptance_test.py"), | ||
), | ||
"_workspace_tmpl": attr.label( | ||
doc = "TODO", | ||
allow_single_file = True, | ||
default = "//python/tests/toolchains/workspace_template:WORKSPACE.tmpl", | ||
default = Label("//python/tests/toolchains/workspace_template:WORKSPACE.tmpl"), | ||
), | ||
}, | ||
test = True, | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,23 +14,49 @@ | |||||||||
|
||||||||||
import os | ||||||||||
import subprocess | ||||||||||
import sys | ||||||||||
import tempfile | ||||||||||
import unittest | ||||||||||
|
||||||||||
|
||||||||||
class TestPythonVersion(unittest.TestCase): | ||||||||||
@classmethod | ||||||||||
def setUpClass(cls): | ||||||||||
os.chdir("%test_location%") | ||||||||||
python_version_test_dirname = os.path.dirname( | ||||||||||
os.path.realpath("python_version_test.py") | ||||||||||
) | ||||||||||
rules_python_path = os.path.normpath( | ||||||||||
os.path.join(python_version_test_dirname, "..", "..", "..", "..") | ||||||||||
) | ||||||||||
|
||||||||||
is_windows = "win" in sys.platform | ||||||||||
if is_windows: | ||||||||||
newline = "\r\n" | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe python has a built in constant for this. Would be nice. |
||||||||||
os.environ["HOME"] = tempfile.mkdtemp() | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that'd be much better since it should show up in sandbox paths, making things more debuggable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
os.environ["LocalAppData"] = tempfile.mkdtemp() | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bazel requires these variables be set or some flags be explicitly provided. This is easier IMO |
||||||||||
else: | ||||||||||
newline = "\n" | ||||||||||
|
||||||||||
with open(".bazelrc", "w") as bazelrc: | ||||||||||
bazelrc.write( | ||||||||||
newline.join( | ||||||||||
[ | ||||||||||
'build --override_repository rules_python="{}"'.format( | ||||||||||
rules_python_path.replace("\\", "/") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bazel can understand Windows paths with forward slashes. |
||||||||||
), | ||||||||||
"build --test_output=errors", | ||||||||||
] | ||||||||||
) | ||||||||||
) | ||||||||||
|
||||||||||
def test_match_toolchain(self): | ||||||||||
stream = os.popen("bazel run @python_toolchain_host//:python3 -- --version") | ||||||||||
output = stream.read() | ||||||||||
self.assertEqual(output, "Python %python_version%\n") | ||||||||||
output = stream.read().strip() | ||||||||||
self.assertEqual(output, "Python %python_version%") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline can be different on different system. Better to just strip whitespace. |
||||||||||
|
||||||||||
subprocess.run("bazel test //...", shell=True, check=True) | ||||||||||
|
||||||||||
|
||||||||||
if __name__ == "__main__": | ||||||||||
os.chdir("%test_location%") | ||||||||||
python_version_test_dirname = os.path.dirname(os.path.realpath("python_version_test.py")) | ||||||||||
rules_python_path = os.path.join(python_version_test_dirname, "..", "..", "..", "..") | ||||||||||
with open(".bazelrc", "w") as bazelrc: | ||||||||||
bazelrc.write("build --override_repository rules_python=\"{}\"\n".format(rules_python_path)) | ||||||||||
bazelrc.write("build --test_output=errors\n") | ||||||||||
unittest.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This flag is required to enable runfiles on windows