feat(protractor): protractor npm package is now a peer deps (#1352)

Also cleanup us arguments and doc string for protractor_web_test_suite.

BREAKING CHANGES:

npm protractor deps for protractor_web_test and protractor_web_test_suite is now a peer dep so that the versions used can be chosen by the user.

e2e/ts_devserver/package.json

@@ -7,6 +7,7 @@
     "@types/node": "7.0.18",
     "date-fns": "1.30.1",
     "jasmine": "2.8.0",
+    "protractor": "^5.4.2",
     "rxjs": "^6.5.2",
     "typeorm": "^0.2.17",
     "typescript": "2.7.x"

examples/angular/package.json

@@ -53,6 +53,7 @@
         "karma-requirejs": "1.1.0",
         "karma-sourcemap-loader": "0.3.7",
         "requirejs": "2.3.6",
+        "protractor": "^5.4.2",
         "rollup": "^1.21.4",
         "rollup-plugin-commonjs": "^10.1.0",
         "rollup-plugin-node-resolve": "^5.2.0",

examples/angular_view_engine/package.json

@@ -53,6 +53,7 @@
         "karma-requirejs": "1.1.0",
         "karma-sourcemap-loader": "0.3.7",
         "patch-package": "^6.2.0",
+        "protractor": "^5.4.2",
         "requirejs": "2.3.6",
         "rollup": "^1.21.4",
         "rollup-plugin-amd": "^4.0.0",

examples/app/package.json

@@ -8,6 +8,7 @@
     "@types/jasmine": "3.3.15",
     "http-server": "^0.11.1",
     "less": "^3.10.3",
+    "protractor": "^5.4.2",
     "rollup": "1.20.3",
     "stylus": "^0.54.7",
     "terser": "4.3.1",

examples/protocol_buffers/package.json

@@ -19,6 +19,7 @@
     "karma-sourcemap-loader": "0.3.7",
     "long": "4.0.0",
     "protobufjs": "5.0.3",
+    "protractor": "^5.4.2",
     "requirejs": "2.3.6",
     "rollup": "1.20.3",
     "terser": "4.3.1",

examples/webapp/package.json

@@ -10,6 +10,7 @@
         "@bazel/typescript": "^0.40.0",
         "http-server": "^0.11.1",
         "mocha": "^6.2.1",
+        "protractor": "^5.4.2",
         "rollup": "1.21.4",
         "source-map": "^0.7.3",
         "terser": "^4.3.1",

packages/protractor/src/BUILD.bazel

@@ -40,7 +40,6 @@ filegroup(
         "package.bzl",
         "package.json",
         "protractor.conf.js",
-        "protractor.js",
         "protractor_web_test.bzl",
     ],
 )

packages/protractor/src/index.from_src.bzl

@@ -21,27 +21,13 @@ load(
     _protractor_web_test_suite = "protractor_web_test_suite",
 )
 
-INTERNAL_PROTRACTOR = "@npm//protractor"
-INTERNAL_PROTRACTOR_ENTRY_POINT = "@npm_bazel_protractor//:protractor.js"
+_PROTRACTOR_PEER_DEPS = [
+    "@npm_bazel_protractor//:utils_lib",
+    "@npm//protractor",
+]
 
 def protractor_web_test(data = [], **kwargs):
-    _protractor_web_test(
-        # When there is no @npm//@bazel/protractor package we use @npm_bazel_protractor instead.
-        # @npm_bazel_protractor//:utils_lib dependency must also be added manually since without a dep on
-        # @npm//@bazel/protractor "@bazel/protractor/protractor-utils" will not resolve.
-        data = data + ["@npm_bazel_protractor//:utils_lib"],
-        protractor = INTERNAL_PROTRACTOR,
-        protractor_entry_point = INTERNAL_PROTRACTOR_ENTRY_POINT,
-        **kwargs
-    )
+    _protractor_web_test(peer_deps = _PROTRACTOR_PEER_DEPS, **kwargs)
 
 def protractor_web_test_suite(data = [], **kwargs):
-    _protractor_web_test_suite(
-        # When there is no @npm//@bazel/protractor package we use @npm_bazel_protractor instead.
-        # @npm_bazel_protractor//:utils_lib dependency must also be added manually since without a dep on
-        # @npm//@bazel/protractor "@bazel/protractor/protractor-utils" will not resolve.
-        data = data + ["@npm_bazel_protractor//:utils_lib"],
-        protractor = INTERNAL_PROTRACTOR,
-        protractor_entry_point = INTERNAL_PROTRACTOR_ENTRY_POINT,
-        **kwargs
-    )
+    _protractor_web_test_suite(peer_deps = _PROTRACTOR_PEER_DEPS, **kwargs)

packages/protractor/src/package.json

@@ -15,9 +15,8 @@
       "protractor",
       "bazel"
   ],
-  "main": "index.js",
-  "dependencies": {
-      "protractor": "^5.4.2"
+  "peerDependencies": {
+      "protractor": ">=5.0.0"
   },
   "bazelWorkspaces": {
       "npm_bazel_protractor": {

packages/protractor/src/protractor_web_test.bzl

@@ -19,9 +19,11 @@ load("@build_bazel_rules_nodejs//internal/common:windows_utils.bzl", "create_win
 load("@io_bazel_rules_webtesting//web:web.bzl", "web_test_suite")
 load("@io_bazel_rules_webtesting//web/internal:constants.bzl", "DEFAULT_WRAPPED_TEST_TAGS")
 
-_CONF_TMPL = "//:protractor.conf.js"
-_DEFUALT_PROTRACTOR = "@npm//@bazel/protractor"
-_DEFUALT_PROTRACTOR_ENTRY_POINT = "@npm//:node_modules/@bazel/protractor/protractor.js"
+_PROTRACTOR_PEER_DEPS = [
+    "@npm//@bazel/protractor",
+    "@npm//protractor",
+]
+_PROTRACTOR_ENTRY_POINT = "@npm//:node_modules/protractor/bin/protractor"
 
 # Avoid using non-normalized paths (workspace/../other_workspace/path)
 def _to_manifest_path(ctx, file):
@@ -195,7 +197,7 @@ _protractor_web_test = rule(
             aspects = [node_modules_aspect],
         ),
         "_conf_tmpl": attr.label(
-            default = Label(_CONF_TMPL),
+            default = Label("//:protractor.conf.js"),
             allow_single_file = True,
         ),
     },
@@ -210,8 +212,8 @@ def protractor_web_test(
         data = [],
         server = None,
         tags = [],
-        protractor = _DEFUALT_PROTRACTOR,
-        protractor_entry_point = _DEFUALT_PROTRACTOR_ENTRY_POINT,
+        peer_deps = _PROTRACTOR_PEER_DEPS,
+        protractor_entry_point = _PROTRACTOR_ENTRY_POINT,
         **kwargs):
     """Runs a protractor test in a browser.
 
@@ -226,17 +228,18 @@ def protractor_web_test(
       data: Runtime dependencies
       server: Optional server executable target
       tags: Standard Bazel tags, this macro adds one for ibazel
-      protractor: A label providing the @bazel/protractor npm dependency.
-      protractor_entry_point: A label providing the @bazel/protractor entry point.
-      **kwargs: passed through to `_protractor_web_test`
+      peer_deps: List of peer npm deps required by protractor_web_test.
+      protractor_entry_point: A label providing the @npm//protractor entry point.
+          Default to `@npm//:node_modules/protractor/bin/protractor`.
+      **kwargs: passed through to `protractor_web_test`
     """
 
     protractor_bin_name = name + "_protractor_bin"
 
     nodejs_binary(
         name = protractor_bin_name,
         entry_point = protractor_entry_point,
-        data = srcs + deps + data + [protractor],
+        data = srcs + deps + data + peer_deps,
         testonly = 1,
         visibility = ["//visibility:private"],
     )
@@ -265,133 +268,76 @@ def protractor_web_test(
 
 def protractor_web_test_suite(
         name,
-        configuration = None,
-        on_prepare = None,
-        srcs = [],
-        deps = [],
-        data = [],
-        server = None,
         browsers = None,
-        args = None,
-        browser_overrides = None,
-        config = None,
-        flaky = None,
-        local = None,
-        shard_count = None,
-        size = None,
-        tags = [],
-        test_suite_tags = None,
-        timeout = None,
-        visibility = None,
         web_test_data = [],
-        wrapped_test_tags = None,
-        protractor = _DEFUALT_PROTRACTOR,
-        protractor_entry_point = _DEFUALT_PROTRACTOR_ENTRY_POINT,
-        **remaining_keyword_args):
+        wrapped_test_tags = list(DEFAULT_WRAPPED_TEST_TAGS),
+        **kwargs):
     """Defines a test_suite of web_test targets that wrap a protractor_web_test target.
 
     Args:
-      name: The base name of the test.
-      configuration: Protractor configuration file.
-      on_prepare: A file with a node.js script to run once before all tests run.
-          If the script exports a function which returns a promise, protractor
-          will wait for the promise to resolve before beginning tests.
-      srcs: JavaScript source files
-      deps: Other targets which produce JavaScript such as `ts_library`
-      data: Runtime dependencies
-      server: Optional server executable target
+      name: The base name of the test
       browsers: A sequence of labels specifying the browsers to use.
-      args: Args for web_test targets generated by this extension.
-      browser_overrides: Dictionary; optional; default is an empty dictionary. A
-        dictionary mapping from browser names to browser-specific web_test
-        attributes, such as shard_count, flakiness, timeout, etc. For example:
-        {'//browsers:chrome-native': {'shard_count': 3, 'flaky': 1}
-         '//browsers:firefox-native': {'shard_count': 1, 'timeout': 100}}.
-      config: Label; optional; Configuration of web test features.
-      flaky: A boolean specifying that the test is flaky. If set, the test will
-        be retried up to 3 times (default: 0)
-      local: boolean; optional.
-      shard_count: The number of test shards to use per browser. (default: 1)
-      size: A string specifying the test size. (default: 'large')
-      tags: A list of test tag strings to apply to each generated web_test target.
-        This macro adds a couple for ibazel.
-      test_suite_tags: A list of tag strings for the generated test_suite.
-      timeout: A string specifying the test timeout (default: computed from size)
-      visibility: List of labels; optional.
-      web_test_data: Data dependencies for the web_test.
-      wrapped_test_tags: A list of test tag strings to use for the wrapped test
-      protractor: Protractor entry_point. Defaults to @npm//:node_modules/protractor/bin/protractor
-          but should be changed to @your_npm_workspace//:node_modules/protractor/bin/protractor if
-          you are not using @npm for your npm dependencies.
-      **remaining_keyword_args: Arguments for the wrapped test target.
+      web_test_data: Data dependencies for the wrapoer web_test targets.
+      wrapped_test_tags: A list of test tag strings to use for the wrapped
+        karma_web_test target.
+      **kwargs: Arguments for the wrapped karma_web_test target.
     """
 
-    # Check explicitly for None so that users can set this to the empty list
-    if wrapped_test_tags == None:
-        wrapped_test_tags = DEFAULT_WRAPPED_TEST_TAGS
-
+    # Common attributes
+    args = kwargs.pop("args", None)
+    flaky = kwargs.pop("flaky", None)
+    local = kwargs.pop("local", None)
+    shard_count = kwargs.pop("shard_count", None)
+    size = kwargs.pop("size", "large")
+    timeout = kwargs.pop("timeout", None)
+
+    # Wrapper attributes
+    browser_overrides = kwargs.pop("browser_overrides", None)
+    config = kwargs.pop("config", None)
+    test_suite_tags = kwargs.pop("test_suite_tags", None)
+    visibility = kwargs.pop("visibility", None)
+    tags = kwargs.pop("tags", []) + [
+        # Users don't need to know that this tag is required to run under ibazel
+        "ibazel_notify_changes",
+    ]
     if browsers == None:
         browsers = ["@io_bazel_rules_webtesting//browsers:chromium-local"]
+
+        # rules_webesting requires the "native" tag for browsers
         if not "native" in tags:
             tags = tags + ["native"]
 
-    size = size or "large"
-
+    # The wrapped `karma_web_test` target
     wrapped_test_name = name + "_wrapped_test"
-    protractor_bin_name = name + "_protractor_bin"
-
-    # Users don't need to know that this tag is required to run under ibazel
-    tags = tags + ["ibazel_notify_changes"]
-
-    nodejs_binary(
-        name = protractor_bin_name,
-        entry_point = protractor_entry_point,
-        data = srcs + deps + data + [protractor],
-        testonly = 1,
-        visibility = ["//visibility:private"],
-    )
-
-    # Our binary dependency must be in data[] for collect_data to pick it up
-    # FIXME: maybe we can just ask the :protractor_bin_name for its runfiles attr
-    web_test_data = web_test_data + [":" + protractor_bin_name]
-    if server:
-        web_test_data += [server]
-
-    _protractor_web_test(
+    protractor_web_test(
         name = wrapped_test_name,
-        configuration = configuration,
-        on_prepare = on_prepare,
-        srcs = srcs,
-        deps = deps,
-        data = web_test_data,
-        server = server,
-        protractor = protractor_bin_name,
         args = args,
         flaky = flaky,
         local = local,
         shard_count = shard_count,
         size = size,
-        tags = wrapped_test_tags,
         timeout = timeout,
+        tags = wrapped_test_tags,
         visibility = ["//visibility:private"],
-        **remaining_keyword_args
+        **kwargs
     )
 
+    # The wrapper `web_test_suite` target
     web_test_suite(
         name = name,
-        launcher = ":" + wrapped_test_name,
         args = args,
-        browsers = browsers,
-        browser_overrides = browser_overrides,
-        config = config,
-        data = web_test_data,
         flaky = flaky,
         local = local,
         shard_count = shard_count,
         size = size,
+        timeout = timeout,
+        launcher = ":" + wrapped_test_name,
+        browsers = browsers,
+        browser_overrides = browser_overrides,
+        config = config,
+        data = web_test_data,
         tags = tags,
         test = wrapped_test_name,
         test_suite_tags = test_suite_tags,
-        timeout = timeout,
         visibility = visibility,
     )

packages/protractor/test/protractor-2/BUILD.bazel

@@ -54,7 +54,6 @@ ts_library(
     tsconfig = ":tsconfig-test",
     deps = [
         "@npm//@types/jasmine",
-        "@npm//@types/selenium-webdriver",
         "@npm//protractor",
     ],
 )

packages/protractor/test/protractor/BUILD.bazel

@@ -8,7 +8,6 @@ ts_library(
     tsconfig = ":tsconfig.json",
     deps = [
         "@npm//@types/jasmine",
-        "@npm//@types/selenium-webdriver",
         "@npm//protractor",
     ],
 )