message: Update KnowledgeBase, InMemoryKnowledgeBase, ActionQueryKnowledgeBase

diff:
@@ -65,7 +65,7 @@ class ActionQueryKnowledgeBase(Action):
text=f"Did not find a valid value for attribute '{attribute_name}' for object '{object_name}'."
)
- def utter_objects(
+ async def utter_objects(
self,
dispatcher: CollectingDispatcher,
object_type: Text,
@@ -84,9 +84,17 @@ class ActionQueryKnowledgeBase(Action):
text=f"Found the following objects of type '{object_type}':"
)
+ if utils.is_coroutine_action(
+ self.knowledge_base.get_representation_function_of_object
+ ):
+ repr_function = await self.knowledge_base.get_representation_function_of_object(
+ object_type
+ )
+ else:
repr_function = self.knowledge_base.get_representation_function_of_object(
object_type
)
+
for i, obj in enumerate(objects, 1):
dispatcher.utter_message(text=f"{i}: {repr_function(obj)}")
else:
@@ -152,7 +160,7 @@ class ActionQueryKnowledgeBase(Action):
object_type = tracker.get_slot(SLOT_OBJECT_TYPE)
if utils.is_coroutine_action(self.knowledge_base.get_attributes_of_object):
object_attributes = await self.knowledge_base.get_attributes_of_object(
- object_type # type: ignore
+ object_type
)
else:
object_attributes = self.knowledge_base.get_attributes_of_object(
@@ -164,7 +172,7 @@ class ActionQueryKnowledgeBase(Action):
attributes = get_attribute_slots(tracker, object_attributes)
# query the knowledge base
if utils.is_coroutine_action(self.knowledge_base.get_objects):
- objects = await self.knowledge_base.get_objects(object_type, attributes) # type: ignore
+ objects = await self.knowledge_base.get_objects(object_type, attributes)
else:
objects = self.knowledge_base.get_objects(object_type, attributes)
@@ -178,7 +186,7 @@ class ActionQueryKnowledgeBase(Action):
if utils.is_coroutine_action(self.knowledge_base.get_key_attribute_of_object):
key_attribute = await self.knowledge_base.get_key_attribute_of_object(
- object_type # type: ignore
+ object_type
)
else:
key_attribute = self.knowledge_base.get_key_attribute_of_object(object_type)
@@ -251,7 +259,7 @@ class ActionQueryKnowledgeBase(Action):
object_representation = repr_function(object_of_interest)
if utils.is_coroutine_action(self.knowledge_base.get_key_attribute_of_object):
key_attribute = await self.knowledge_base.get_key_attribute_of_object(
- object_type # type: ignore
+ object_type
)
else:
key_attribute = self.knowledge_base.get_key_attribute_of_object(object_type)

diff:
@@ -5,6 +5,8 @@ import random
from typing import Text, Callable, Dict, List, Any, Optional
from collections import defaultdict
+from rasa_sdk import utils
+
logger = logging.getLogger(__name__)
@@ -29,7 +31,7 @@ class KnowledgeBase:
self.key_attribute = defaultdict(lambda: "id")
self.representation_function = defaultdict(lambda: lambda obj: obj["name"])
- def get_attributes_of_object(self, object_type: Text) -> List[Text]:
+ async def get_attributes_of_object(self, object_type: Text) -> List[Text]:
"""
Returns a list of all attributes that belong to the provided object type.
@@ -40,7 +42,7 @@ class KnowledgeBase:
"""
raise NotImplementedError("Method is not implemented.")
- def get_key_attribute_of_object(self, object_type: Text) -> Text:
+ async def get_key_attribute_of_object(self, object_type: Text) -> Text:
"""
Returns the key attribute for the given object type.
@@ -51,7 +53,9 @@ class KnowledgeBase:
"""
return self.key_attribute[object_type]
- def get_representation_function_of_object(self, object_type: Text) -> Callable:
+ async def get_representation_function_of_object(
+ self, object_type: Text
+ ) -> Callable:
"""
Returns a lamdba function that takes the object and returns a string
representation of it.
@@ -73,7 +77,7 @@ class KnowledgeBase:
"""
self.ordinal_mention_mapping = mapping
- def get_objects(
+ async def get_objects(
self, object_type: Text, attributes: List[Dict[Text, Text]], limit: int = 5
) -> List[Dict[Text, Any]]:
"""
@@ -89,7 +93,9 @@ class KnowledgeBase:
"""
raise NotImplementedError("Method is not implemented.")
- def get_object(self, object_type: Text, object_identifier: Text) -> Dict[Text, Any]:
+ async def get_object(
+ self, object_type: Text, object_identifier: Text
+ ) -> Dict[Text, Any]:
"""
Returns the object of the given type that matches the given object identifier.
@@ -158,7 +164,7 @@ class InMemoryKnowledgeBase(KnowledgeBase):
"""
self.key_attribute[object_type] = key_attribute
- def get_attributes_of_object(self, object_type: Text) -> List[Text]:
+ async def get_attributes_of_object(self, object_type: Text) -> List[Text]:
if object_type not in self.data or not self.data[object_type]:
return []
@@ -166,7 +172,7 @@ class InMemoryKnowledgeBase(KnowledgeBase):
return list(first_object.keys())
- def get_objects(
+ async def get_objects(
self, object_type: Text, attributes: List[Dict[Text, Text]], limit: int = 5
) -> List[Dict[Text, Any]]:
if object_type not in self.data:
@@ -190,13 +196,17 @@ class InMemoryKnowledgeBase(KnowledgeBase):
return objects[:limit]
- def get_object(
+ async def get_object(
self, object_type: Text, object_identifier: Text
) -> Optional[Dict[Text, Any]]:
if object_type not in self.data:
return None
objects = self.data[object_type]
+
+ if utils.is_coroutine_action(self.get_key_attribute_of_object):
+ key_attribute = await self.get_key_attribute_of_object(object_type)
+ else:
key_attribute = self.get_key_attribute_of_object(object_type)
# filter the objects by its key attribute, for example, 'id'
@@ -211,7 +221,13 @@ class InMemoryKnowledgeBase(KnowledgeBase):
# if the object was referred to directly, we need to compare the representation
# of each object with the given object identifier
if not objects_of_interest:
+ if utils.is_coroutine_action(self.get_representation_function_of_object):
+ repr_function = await self.get_representation_function_of_object(
+ object_type
+ )
+ else:
repr_function = self.get_representation_function_of_object(object_type)
+
objects_of_interest = list(
filter(
lambda obj: str(object_identifier).lower()

diff:
import pytest
-import asyncio
from rasa_sdk import Tracker
from rasa_sdk.events import SlotSet
@@ -117,16 +116,14 @@ def compare_slots(slot_list_1, slot_list_2):
),
],
)
-def test_action_run(data_file, slots, expected_slots):
+async def test_action_run(data_file, slots, expected_slots):
knowledge_base = InMemoryKnowledgeBase(data_file)
action = ActionQueryKnowledgeBase(knowledge_base)
dispatcher = CollectingDispatcher()
tracker = Tracker("default", slots, {}, [], False, None, {}, "action_listen")
- loop = asyncio.new_event_loop()
- actual_slots = loop.run_until_complete(action.run(dispatcher, tracker, {}))
- loop.close()
+ actual_slots = await action.run(dispatcher, tracker, {})
compare_slots(expected_slots, actual_slots)
compare_slots(actual_slots, expected_slots)

diff:
@@ -12,10 +12,10 @@ from rasa_sdk.knowledge_base.storage import InMemoryKnowledgeBase
("restaurant", [{"name": "cuisine", "value": "Italian"}], 2),
],
)
-def test_query_entities(data_file, object_type, attributes, expected_length):
+async def test_query_entities(data_file, object_type, attributes, expected_length):
knowledge_base = InMemoryKnowledgeBase(data_file)
- entities = knowledge_base.get_objects(
+ entities = await knowledge_base.get_objects(
object_type=object_type, attributes=attributes
)
assert expected_length == len(entities)
@@ -43,10 +43,10 @@ def test_query_entities(data_file, object_type, attributes, expected_length):
("hotel", None, None),
],
)
-def test_query_object(data_file, object_type, object_identifier, expected_value):
+async def test_query_object(data_file, object_type, object_identifier, expected_value):
knowledge_base = InMemoryKnowledgeBase(data_file)
- actual_value = knowledge_base.get_object(
+ actual_value = await knowledge_base.get_object(
object_type=object_type, object_identifier=object_identifier
)
@@ -57,10 +57,12 @@ def test_query_object(data_file, object_type, object_identifier, expected_value)
"object_type,expected_attributes",
[("restaurant", ["id", "name", "cuisine", "wifi"])],
)
-def test_get_attributes_for(data_file, object_type, expected_attributes):
+async def test_get_attributes_for(data_file, object_type, expected_attributes):
knowledge_base = InMemoryKnowledgeBase(data_file)
- actual_attributes = knowledge_base.get_attributes_of_object(object_type=object_type)
+ actual_attributes = await knowledge_base.get_attributes_of_object(
+ object_type=object_type
+ )
assert set(expected_attributes) == set(actual_attributes)
@@ -69,14 +71,14 @@ def test_get_attributes_for(data_file, object_type, expected_attributes):
"object_type,set_key_attribute,expected_key_attribute",
[("restaurant", None, "id"), ("restaurant", "name", "name")],
)
-def test_key_attribute_of_object(
+async def test_key_attribute_of_object(
data_file, object_type, set_key_attribute, expected_key_attribute
):
knowledge_base = InMemoryKnowledgeBase(data_file)
if set_key_attribute:
knowledge_base.set_key_attribute_of_object(object_type, set_key_attribute)
- actual_key_attribute = knowledge_base.get_key_attribute_of_object(
+ actual_key_attribute = await knowledge_base.get_key_attribute_of_object(
object_type=object_type
)
@@ -94,7 +96,7 @@ def test_key_attribute_of_object(
),
],
)
-def test_get_representation_function_of_object(
+async def test_get_representation_function_of_object(
data_file, object_type, set_repr_function, expected_repr_function
):
knowledge_base = InMemoryKnowledgeBase(data_file)
@@ -103,10 +105,10 @@ def test_get_representation_function_of_object(
object_type, set_repr_function
)
- actual_repr_function = knowledge_base.get_representation_function_of_object(
+ actual_repr_function = await knowledge_base.get_representation_function_of_object(
object_type=object_type
)
- dummy_object = knowledge_base.get_object(object_type, 1)
+ dummy_object = await knowledge_base.get_object(object_type, 1)
assert expected_repr_function(dummy_object) == actual_repr_function(dummy_object)


message: Fix PR feedback

change_type: MODIFY

old_path: .gitignore

new_path: .gitignore

diff:
@@ -111,4 +111,3 @@ examples/moodbot/models/
.dir-locals.el
*.DS_Store
-

diff:
@@ -7,7 +7,7 @@ This project adheres to `Semantic Versioning`_ starting with version 0.11.0.
.. _master-release:
-[Unreleased 1.6.2a1] - `master`_
+[Unreleased 1.7.0] - `master`_
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
.. note:: This version is not yet released and is under active development.

diff:
@@ -1130,5 +1130,4 @@ async def test_submit(form_class: Type[FormAction]):
form = form_class()
events = await form.run(dispatcher=None, tracker=tracker, domain=None)
- print(events)
assert events[0]["value"] == 42

license: Apache License 2.0

repo: rasahq/rasa-sdk

original_message: Fix PR feedback


message: Prepare 1.7.0 release

diff:
@@ -6,12 +6,14 @@ This project adheres to `Semantic Versioning`_ starting with version 0.11.0.
.. _master-release:
-
-[Unreleased 1.7.0] - `master`_
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+[Unreleased 1.7.1a1] - `master`_
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
.. note:: This version is not yet released and is under active development.
+[1.7.0] - 2020-01-29
+^^^^^^^^^^^^^^^^^^^^
+
Changed
-------
- ``ReminderScheduled`` and ``ReminderCancelled`` now take ``intent`` and ``entities``

original_message: Prepare 1.7.0 release


message: Add auto-reload upon file timestamp change

diff:
@@ -18,6 +18,7 @@ def main_from_args(args):
args.ssl_certificate,
args.ssl_keyfile,
args.ssl_password,
+ args.auto_reload,
)

diff:
@@ -49,3 +49,8 @@ def add_endpoint_arguments(parser):
help="If your ssl-keyfile is protected by a password, you can specify it "
"using this paramer.",
)
+ parser.add_argument(
+ "--auto-reload",
+ help="Enable auto-reloading of modules containing Action subclasses.",
+ action="store_true",
+ )

diff:
@@ -59,6 +59,7 @@ def create_argument_parser():
def create_app(
action_package_name: Union[Text, types.ModuleType],
cors_origins: Union[Text, List[Text], None] = "*",
+ auto_reload: bool = False,
) -> Sanic:
app = Sanic(__name__, configure_logging=False)
@@ -82,6 +83,10 @@ def create_app(
return response.json(body, status=400)
utils.check_version_compatibility(action_call.get("version"))
+
+ if auto_reload:
+ executor.reload()
+
try:
result = await executor.run(action_call)
except ActionExecutionRejection as e:
@@ -98,6 +103,9 @@ def create_app(
@app.get("/actions")
async def actions(_) -> HTTPResponse:
"""List all registered actions."""
+ if auto_reload:
+ executor.reload()
+
body = [{"name": k} for k in executor.actions.keys()]
return response.json(body, status=200)
@@ -111,9 +119,12 @@ def run(
ssl_certificate: Optional[Text] = None,
ssl_keyfile: Optional[Text] = None,
ssl_password: Optional[Text] = None,
+ auto_reload: bool = False,
):
logger.info("Starting action endpoint server...")
- app = create_app(action_package_name, cors_origins=cors_origins)
+ app = create_app(
+ action_package_name, cors_origins=cors_origins, auto_reload=auto_reload
+ )
ssl_context = create_ssl_context(ssl_certificate, ssl_keyfile, ssl_password)
app.run("0.0.0.0", port, ssl=ssl_context, workers=utils.number_of_sanic_workers())

diff:
@@ -7,14 +7,12 @@ from typing import Text, List, Dict, Any, Type, Union, Callable, Optional
import typing
import types
import sys
+import os
-from rasa_sdk.interfaces import Tracker, ActionNotFoundException
+from rasa_sdk.interfaces import Tracker, ActionNotFoundException, Action
from rasa_sdk import utils
-if typing.TYPE_CHECKING:
- from rasa_sdk.interfaces import Action
-
logger = logging.getLogger(__name__)
@@ -146,16 +144,25 @@ class CollectingDispatcher:
class ActionExecutor:
def __init__(self):
self.actions = {}
+ self._modules = {}
def register_action(self, action: Union[Type["Action"], "Action"]) -> None:
- from rasa_sdk.interfaces import Action
-
if inspect.isclass(action):
if action.__module__.startswith("rasa."):
logger.warning(f"Skipping built in Action {action}.")
return
else:
+ if getattr(action, "_sdk_loaded", False):
+ # This Action subclass has already been loaded in the past;
+ # do not try to load it again as either 1) it is already
+ # loaded or 2) it has been replaced by a newer version
+ # already.
+ return
+
+ # Mark the class as "loaded"
+ action._sdk_loaded = True
action = action()
+
if isinstance(action, Action):
self.register_function(action.name(), action.run)
else:
@@ -166,7 +173,6 @@ class ActionExecutor:
)
def register_function(self, name: Text, f: Callable) -> None:
- logger.info(f"Registered function for '{name}'.")
valid_keys = utils.arguments_of(f)
if len(valid_keys) < 3:
raise Exception(
@@ -176,6 +182,12 @@ class ActionExecutor:
f"tracker, domain. Your function accepts only {len(valid_keys)} "
"parameters."
)
+
+ if name in self.actions:
+ logger.info(f"Re-registered function for '{name}'.")
+ else:
+ logger.info(f"Registered function for '{name}'.")
+
self.actions[name] = f
def _import_submodules(
@@ -189,27 +201,38 @@ class ActionExecutor:
:rtype: dict[str, types.ModuleType]
"""
if isinstance(package, str):
- package = importlib.import_module(package)
+ package = self._import_module(package)
if not getattr(package, "__path__", None):
return
- results = {}
for loader, name, is_pkg in pkgutil.walk_packages(package.__path__):
full_name = package.__name__ + "." + name
- results[full_name] = importlib.import_module(full_name)
+ self._import_module(full_name)
+
if recursive and is_pkg:
self._import_submodules(full_name)
- def register_package(self, package: Union[Text, types.ModuleType]) -> None:
- from rasa_sdk.interfaces import Action
+ def _import_module(self, name: Text) -> types.ModuleType:
+ """TODO
+ """
+ module = importlib.import_module(name)
+
+ timestamp = os.path.getmtime(module.__file__)
+ self._modules[module.__file__] = (timestamp, module)
+ return module
+
+ def register_package(self, package: Union[Text, types.ModuleType]) -> None:
try:
self._import_submodules(package)
except ImportError:
logger.exception(f"Failed to register package '{package}'.")
sys.exit(1)
+ self._register_all_actions()
+
+ def _register_all_actions(self) -> None:
actions = utils.all_subclasses(Action)
for action in actions:
@@ -224,6 +247,40 @@ class ActionExecutor:
):
self.register_action(action)
+ def reload(self) -> None:
+ to_reload = {}
+
+ for path, (timestamp, module) in self._modules.items():
+ try:
+ new_timestamp = os.path.getmtime(path)
+ except OSError:
+ # Ignore missing files
+ continue
+
+ if new_timestamp > timestamp:
+ to_reload[path] = (new_timestamp, module)
+
+ module_reloaded = False
+
+ for path, (timestamp, module) in to_reload.items():
+ try:
+ new_module = importlib.reload(module)
+ self._modules[path] = (timestamp, new_module)
+ logger.info(
+ f"Reloaded module/package: '{module.__name__}' "
+ f"(file: '{os.path.relpath(path)}')"
+ )
+ module_reloaded = True
+ except (SyntaxError, ImportError):
+ logger.exception(
+ f"Error while reloading module/package: '{module.__name__}' "
+ f"(file: '{os.path.relpath(path)}'):"
+ )
+ logger.info("Please fix the error(s) in the Python file and try again.")
+
+ if module_reloaded:
+ self._register_all_actions()
+
@staticmethod
def _create_api_response(
events: List[Dict[Text, Any]], messages: List[Dict[Text, Any]]


message: Add tests for action modules loading.

diff:
@@ -111,3 +111,4 @@ examples/moodbot/models/
.dir-locals.el
*.DS_Store
+tests/executor_test_packages

diff:
@@ -218,6 +218,10 @@ class ActionExecutor:
"""
module = importlib.import_module(name)
+ if module.__file__:
+ # If the module we're importing is a namespace package (a package
+ # without __init__.py), then there's nothing to watch for the
+ # package itself.
timestamp = os.path.getmtime(module.__file__)
self._modules[module.__file__] = (timestamp, module)
@@ -233,17 +237,17 @@ class ActionExecutor:
self._register_all_actions()
def _register_all_actions(self) -> None:
+ import inspect
+
actions = utils.all_subclasses(Action)
for action in actions:
- meta = action.__dict__.get("Meta", False)
- abstract = getattr(meta, "abstract", False)
if (
not action.__module__.startswith("rasa_core.")
and not action.__module__.startswith("rasa.")
and not action.__module__.startswith("rasa_sdk.")
and not action.__module__.startswith("rasa_core_sdk.")
- and not abstract
+ and not inspect.isabstract(action)
):
self.register_action(action)

diff:
@@ -2,12 +2,11 @@ from typing import List, Dict, Text, Any
from rasa_sdk import Action, Tracker
from rasa_sdk.events import SlotSet
-from rasa_sdk.executor import ActionExecutor, CollectingDispatcher
+from rasa_sdk.executor import CollectingDispatcher
from rasa_sdk.utils import is_coroutine_action
class CustomAsyncAction(Action):
- @classmethod
def name(cls) -> Text:
return "custom_async_action"
@@ -20,30 +19,7 @@ class CustomAsyncAction(Action):
return [SlotSet("test", "foo"), SlotSet("test2", "boo")]
-class CustomActionBase(Action):
- @classmethod
- def name(cls) -> Text:
- # Name method needed to test if base action was registered
- return "base_action"
-
- class Meta:
- abstract = True
-
- @staticmethod
- def some_common_feature() -> Text:
- return "test"
-
- def run(
- self,
- dispatcher: CollectingDispatcher,
- tracker: Tracker,
- domain: Dict[Text, Any],
- ) -> List[Dict[Text, Any]]:
- raise NotImplementedError
-
-
-class CustomAction(CustomActionBase):
- @classmethod
+class CustomAction(Action):
def name(cls) -> Text:
return "custom_action"
@@ -53,23 +29,7 @@ class CustomAction(CustomActionBase):
tracker: Tracker,
domain: Dict[Text, Any],
) -> List[Dict[Text, Any]]:
- return [SlotSet("test", self.some_common_feature())]
-
-
-def test_action_registration():
- executor = ActionExecutor()
- executor.register_package("tests")
- assert CustomAction.name() in executor.actions
- assert CustomActionBase.name() not in executor.actions
-
-
-def test_abstract_action():
- dispatcher = CollectingDispatcher()
- tracker = Tracker("test", {}, {}, [], False, None, {}, "listen")
- domain = {}
-
- events = CustomAction().run(dispatcher, tracker, domain)
- assert events == [SlotSet("test", "test")]
+ return [SlotSet("test", "bar")]
def test_action_async_check():

diff:
@@ -42,7 +42,7 @@ def test_server_webhook_custom_action_returns_200():
request, response = app.test_client.post("/webhook", data=json.dumps(data))
events = response.json.get("events")
- assert events == [SlotSet("test", "test")]
+ assert events == [SlotSet("test", "bar")]
assert response.status == 200

diff:
-import pytest
+import os
+import shutil
+import random
+import string
+import time
+from typing import Text
+import pytest
from rasa_sdk.executor import ActionExecutor, CollectingDispatcher
+TEST_PACKAGE_BASE = "tests/executor_test_packages"
+
+ACTION_TEMPLATE = """
+from rasa_sdk import Action
+
+class {class_name}(Action):
+ def name(self):
+ return "{action_name}"
+
+ async def run(self, dispatcher, tracker, domain):
+ dispatcher.utter_message("{message}")
+ return []
+"""
+
+
+def _write_action_file(
+ package_path: Text,
+ file_name: Text,
+ class_name: Text,
+ action_name: Text,
+ message: Text = None,
+) -> None:
+ with open(os.path.join(package_path, file_name), "w") as f:
+ f.write(
+ ACTION_TEMPLATE.format(
+ class_name=class_name, action_name=action_name, message=message
+ )
+ )
+
@pytest.mark.parametrize(
"noevent", [1, ["asd"], "asd", 3.2, None, {"event_without_event_property": 4}]
@@ -30,3 +65,135 @@ def test_deprecated_utter_elements():
"image": None,
"attachment": None,
}
+
+
+@pytest.fixture()
+def package_path() -> None:
+ # Create an empty directory somewhere reached by the Python import search
+ # paths
+ name = "".join(random.choice(string.ascii_lowercase) for _ in range(10))
+ package_path = os.path.join(TEST_PACKAGE_BASE, name)
+
+ # Ensure package dir is clean
+ shutil.rmtree(package_path, ignore_errors=True)
+ os.makedirs(package_path)
+ yield package_path
+
+ # Cleanup
+ shutil.rmtree(package_path, ignore_errors=True)
+
+
+@pytest.fixture()
+def executor() -> ActionExecutor:
+ return ActionExecutor()
+
+
+@pytest.fixture()
+def dispatcher() -> CollectingDispatcher:
+ return CollectingDispatcher()
+
+
+def test_load_action_from_init(executor: ActionExecutor, package_path: Text):
+ # Actions should be loaded from packages
+ _write_action_file(package_path, "__init__.py", "InitAction", "init_action")
+ executor.register_package(package_path.replace("/", "."))
+
+ assert "init_action" in executor.actions
+
+
+def test_load_submodules_in_package(executor: ActionExecutor, package_path: Text):
+ # If there's submodules inside a package, they should be picked up correctly
+ _write_action_file(package_path, "__init__.py", "Action1", "action1")
+ _write_action_file(package_path, "a1.py", "Action2", "action2")
+ _write_action_file(package_path, "a2.py", "Action3", "action3")
+
+ executor.register_package(package_path.replace("/", "."))
+
+ for name in ["action1", "action2", "action3"]:
+ assert name in executor.actions
+
+
+def test_load_submodules_in_namespace_package(
+ executor: ActionExecutor, package_path: Text
+):
+ # If there's submodules inside a namespace package (a package without
+ # __init__.py), they should be picked up correctly
+ _write_action_file(package_path, "foo.py", "FooAction", "foo_action")
+ _write_action_file(package_path, "bar.py", "BarAction", "bar_action")
+
+ executor.register_package(package_path.replace("/", "."))
+
+ for name in ["foo_action", "bar_action"]:
+ assert name in executor.actions
+
+
+def test_load_module_directly(executor: ActionExecutor, package_path: Text):
+ _write_action_file(
+ package_path, "test_module.py", "TestModuleAction", "test_module_action"
+ )
+
+ # Load a module directly, not a package. This is equivalent to loading
+ # "action" when there's an "action.py" file in the module search path.
+ executor.register_package(package_path.replace("/", ".") + ".test_module")
+
+ assert "test_module_action" in executor.actions
+
+
+async def test_reload_module(
+ executor: ActionExecutor, dispatcher: CollectingDispatcher, package_path: Text
+):
+ action_class = "MyAction"
+ action_file = "my_action.py"
+ action_name = "my_action"
+
+ _write_action_file(
+ package_path, action_file, action_class, action_name, message="foobar"
+ )
+
+ executor.register_package(package_path.replace("/", "."))
+
+ action_v1 = executor.actions.get(action_name)
+ assert action_v1
+
+ await action_v1(dispatcher, None, None)
+
+ assert dispatcher.messages[0] == {
+ "text": "foobar",
+ "buttons": [],
+ "elements": [],
+ "custom": {},
+ "template": None,
+ "image": None,
+ "attachment": None,
+ }
+
+ # Write the action file again, but change its contents
+ _write_action_file(
+ package_path, action_file, action_class, action_name, message="hello!"
+ )
+
+ # Manually set the file's timestamp 10 seconds into the future, otherwise
+ # Python will load the class already compiled in __pycache__, which is the
+ # previous version.
+ mod_time = time.time() + 10
+ os.utime(os.path.join(package_path, action_file), times=(mod_time, mod_time))
+
+ # Reload modules
+ executor.reload()
+ dispatcher.messages.clear()
+
+ action_v2 = executor.actions.get(action_name)
+ assert action_v2
+
+ await action_v2(dispatcher, None, None)
+
+ # The message should have changed
+ assert dispatcher.messages[0] == {
+ "text": "hello!",
+ "buttons": [],
+ "elements": [],
+ "custom": {},
+ "template": None,
+ "image": None,
+ "attachment": None,
+ }

original_message: Add tests for action modules loading.


message: Fix tests on Python 3.6

diff:
@@ -224,12 +224,13 @@ class ActionExecutor:
"""
module = importlib.import_module(name)
- if module.__file__:
+ module_file = getattr(module, "__file__", None)
+ if module_file:
# If the module we're importing is a namespace package (a package
# without __init__.py), then there's nothing to watch for the
# package itself.
- timestamp = os.path.getmtime(module.__file__)
- self._modules[module.__file__] = (timestamp, module)
+ timestamp = os.path.getmtime(module_file)
+ self._modules[module_file] = (timestamp, module)
return module

original_message: Fix tests on Python 3.6


message: Fix PR feedback.

diff:
@@ -61,6 +61,17 @@ def create_app(
cors_origins: Union[Text, List[Text], None] = "*",
auto_reload: bool = False,
) -> Sanic:
+ """Create a Sanic application and return it.
+
+ Args:
+ action_package_name: Name of the package or module to load actions
+ from.
+ cors_origins: CORS origins to allow.
+ auto_reload: When `True`, auto-reloading of actions is enabled.
+
+ Returns:
+ A new Sanic application ready to be run.
+ """
app = Sanic(__name__, configure_logging=False)
configure_cors(app, cors_origins)

diff:
@@ -4,6 +4,7 @@ import logging
import pkgutil
import warnings
from typing import Text, List, Dict, Any, Type, Union, Callable, Optional
+from collections import namedtuple
import typing
import types
import sys
@@ -141,10 +142,12 @@ class CollectingDispatcher:
self.utter_message(image=image, **kwargs)
+TimestampModule = namedtuple("TimestampModule", ["timestamp", "module"])
+
class ActionExecutor:
def __init__(self):
self.actions = {}
- self._modules = {}
+ self._modules: Dict[Text, TimestampModule] = {}
def register_action(self, action: Union[Type["Action"], "Action"]) -> None:
if inspect.isclass(action):
@@ -230,7 +233,7 @@ class ActionExecutor:
# without __init__.py), then there's nothing to watch for the
# package itself.
timestamp = os.path.getmtime(module_file)
- self._modules[module_file] = (timestamp, module)
+ self._modules[module_file] = TimestampModule(timestamp, module)
return module
@@ -238,6 +241,9 @@ class ActionExecutor:
"""Register all the `Action` subclasses contained in a Python module or
package.
+ If an `ImportError` is raised when loading the module or package, the
+ action server is stopped with exit code 1.
+
Args:
package: Module or package name, or an already loaded Python module.
"""
@@ -266,18 +272,15 @@ class ActionExecutor:
):
self.register_action(action)
- def reload(self) -> None:
- """Reload all Python modules that have been loaded in the past.
-
- To check if a module should be reloaded, the file's last timestamp is
- checked against the current one. If the current last-modified timestamp
- is larger than the previous one, the module is re-imported using
- `importlib.reload`.
+ def _find_modules_to_reload(self) -> Dict[Text, TimestampModule]:
+ """Finds all Python modules that should be reloaded by checking their
+ files' timestamps.
- If one or more modules are reloaded during this process, the entire
- `Action` class hierarchy is scanned again to see what new classes can
- be registered.
+ Returns:
+ Dictionary containing file paths, new timestamps and Python modules
+ that should be reloaded.
"""
+
to_reload = {}
for path, (timestamp, module) in self._modules.items():
@@ -288,19 +291,34 @@ class ActionExecutor:
continue
if new_timestamp > timestamp:
- to_reload[path] = (new_timestamp, module)
+ to_reload[path] = TimestampModule(new_timestamp, module)
- module_reloaded = False
+ return to_reload
+
+ def reload(self) -> None:
+ """Reload all Python modules that have been loaded in the past.
+
+ To check if a module should be reloaded, the file's last timestamp is
+ checked against the current one. If the current last-modified timestamp
+ is larger than the previous one, the module is re-imported using
+ `importlib.reload`.
+
+ If one or more modules are reloaded during this process, the entire
+ `Action` class hierarchy is scanned again to see what new classes can
+ be registered.
+ """
+ to_reload = self._find_modules_to_reload()
+ any_module_reloaded = False
for path, (timestamp, module) in to_reload.items():
try:
new_module = importlib.reload(module)
- self._modules[path] = (timestamp, new_module)
+ self._modules[path] = TimestampModule(timestamp, new_module)
logger.info(
f"Reloaded module/package: '{module.__name__}' "
f"(file: '{os.path.relpath(path)}')"
)
- module_reloaded = True
+ any_module_reloaded = True
except (SyntaxError, ImportError):
logger.exception(
f"Error while reloading module/package: '{module.__name__}' "
@@ -308,7 +326,7 @@ class ActionExecutor:
)
logger.info("Please fix the error(s) in the Python file and try again.")
- if module_reloaded:
+ if any_module_reloaded:
self._register_all_actions()
@staticmethod

diff:
@@ -3,7 +3,7 @@ import shutil
import random
import string
import time
-from typing import Text
+from typing import Text, Optional, Generator
import pytest
from rasa_sdk.executor import ActionExecutor, CollectingDispatcher
@@ -28,7 +28,7 @@ def _write_action_file(
file_name: Text,
class_name: Text,
action_name: Text,
- message: Text = None,
+ message: Optional[Text] = None,
) -> None:
with open(os.path.join(package_path, file_name), "w") as f:
f.write(
@@ -68,7 +68,7 @@ def test_deprecated_utter_elements():
@pytest.fixture()
-def package_path() -> None:
+def package_path() -> Generator[Text, None, Text]:
# Create an empty directory somewhere reached by the Python import search
# paths
name = "".join(random.choice(string.ascii_lowercase) for _ in range(10))

original_message: Fix PR feedback.


message: Update rasa_sdk/endpoint.py

diff:
@@ -131,7 +131,7 @@ def run(
ssl_keyfile: Optional[Text] = None,
ssl_password: Optional[Text] = None,
auto_reload: bool = False,
-):
+) -> NoReturn:
logger.info("Starting action endpoint server...")
app = create_app(
action_package_name, cors_origins=cors_origins, auto_reload=auto_reload

original_message:
Update rasa_sdk/endpoint.py

Co-Authored-By: Tobias Wochinger <t.wochinger@rasa.com>


message: Change return type

diff:
import argparse
import logging
import types
-from typing import List, Text, Union, Optional, Any, NoReturn
+from typing import List, Text, Union, Optional, Any
from ssl import SSLContext
from sanic import Sanic, response
@@ -131,7 +131,7 @@ def run(
ssl_keyfile: Optional[Text] = None,
ssl_password: Optional[Text] = None,
auto_reload: bool = False,
-) -> NoReturn:
+) -> None:
logger.info("Starting action endpoint server...")
app = create_app(
action_package_name, cors_origins=cors_origins, auto_reload=auto_reload

original_message: Change return type


message: Fix ActionExecutor not loading some subclasses of Action

diff:
@@ -3,7 +3,7 @@ import inspect
import logging
import pkgutil
import warnings
-from typing import Text, List, Dict, Any, Type, Union, Callable, Optional
+from typing import Text, List, Dict, Any, Type, Union, Callable, Optional, Set
from collections import namedtuple
import typing
import types
@@ -149,6 +149,7 @@ class ActionExecutor:
def __init__(self):
self.actions = {}
self._modules: Dict[Text, TimestampModule] = {}
+ self._loaded: Set[Type] = set()
def register_action(self, action: Union[Type["Action"], "Action"]) -> None:
if inspect.isclass(action):
@@ -156,15 +157,12 @@ class ActionExecutor:
logger.warning(f"Skipping built in Action {action}.")
return
else:
- if getattr(action, "_sdk_loaded", False):
- # This Action subclass has already been loaded in the past;
- # do not try to load it again as either 1) it is already
- # loaded or 2) it has been replaced by a newer version
- # already.
+ # Check if this class has already been loaded in the past.
+ if action in self._loaded:
return
# Mark the class as "loaded"
- action._sdk_loaded = True
+ self._loaded.add(action)
action = action()
if isinstance(action, Action):

diff:
@@ -3,6 +3,8 @@ import shutil
import random
import string
import time
+
+from rasa_sdk import Action
from typing import Text, Optional, Generator
import pytest
@@ -197,3 +199,24 @@ async def test_reload_module(
"image": None,
"attachment": None,
}
+
+
+class SubclassTestActionA(Action):
+ def name(self):
+ return "subclass_test_action_a"
+
+
+class SubclassTestActionB(SubclassTestActionA):
+ def name(self):
+ return "subclass_test_action_b"
+
+
+def test_load_subclasses(executor: ActionExecutor):
+ executor.register_action(SubclassTestActionB)
+ assert list(executor.actions) == ["subclass_test_action_b"]
+
+ executor.register_action(SubclassTestActionA)
+ assert sorted(list(executor.actions)) == [
+ "subclass_test_action_a",
+ "subclass_test_action_b",
+ ]


message: prepared release of version 1.10.1

diff:
@@ -13,6 +13,14 @@ This project adheres to `Semantic Versioning`_ starting with version 0.11.0.
.. towncrier release notes start
+[1.10.1] - 2020-05-11
+^^^^^^^^^^^^^^^^^^^^^
+
+Bugfixes
+--------
+- `#212 <https://github.com/rasahq/rasa/issues/212>`_: Fix ``ActionExecutor`` sometimes not loading user-defined actions that inherit from other user-defined ones.
+
+
[1.10.0] - 2020-04-28
^^^^^^^^^^^^^^^^^^^^^

change_type: DELETE

diff: -Fix ``ActionExecutor`` sometimes not loading user-defined actions that inherit from other user-defined ones.

diff:
@@ -4,7 +4,7 @@ build-backend = "poetry.masonry.api"
[tool.poetry]
name = "rasa-sdk"
-version = "1.10.0"
+version = "1.10.1"
description = "Open source machine learning framework to automate text- and voice-based conversations: NLU, dialogue management, connect to Slack, Facebook, and more - Create chatbots and voice assistants"
authors = [ "Rasa Technologies GmbH <hi@rasa.com>",]
maintainers = [ "Tom Bocklisch <tom@rasa.com>",]

diff:
# this file will automatically be changed,
# do not add anything but the version number here!
-__version__ = "1.10.0"
+__version__ = "1.10.1"

original_message: prepared release of version 1.10.1


message: added method in utils for configuring loggging in file

diff:
@@ -85,6 +85,25 @@ def configure_colored_logging(loglevel):
)
+def configure_file_logging(
+ logger_obj: logging.Logger, log_file: Optional[Text]
+) -> None:
+ """Configure logging to a file.
+
+ Args:
+ logger_obj: Logger object to configure.
+ log_file: Path of log file to write to.
+ """
+ if not log_file:
+ return
+
+ formatter = logging.Formatter("%(asctime)s [%(levelname)-5.5s] %(message)s")
+ file_handler = logging.FileHandler(log_file, encoding="utf-8")
+ file_handler.setLevel(logger_obj.level)
+ file_handler.setFormatter(formatter)
+ logger_obj.addHandler(file_handler)
+
+
def arguments_of(func) -> AbstractSet[Text]:
"""Return the parameters of the function `func` as a list of their names."""


message: added method in utils for adding new parser argument for mentioning log-file

diff:
@@ -69,6 +69,15 @@ def add_logging_option_arguments(parser):
)
+def add_logging_file_arguments(parser):
+ parser.add_argument(
+ "--log-file",
+ type=str,
+ default=None,
+ help="Store logs in specified file.",
+ )
+
+
def configure_colored_logging(loglevel):
import coloredlogs


message: renaming add_logging_option_arguments method to add_logging_level_option_arguments, adding logging file arguments in parset in endpoint.py

diff:
@@ -52,7 +52,8 @@ def create_argument_parser():
parser = argparse.ArgumentParser(description="starts the action endpoint")
add_endpoint_arguments(parser)
- utils.add_logging_option_arguments(parser)
+ utils.add_logging_level_option_arguments(parser)
+ utils.add_logging_file_arguments(parser)
return parser

diff:
@@ -39,7 +39,7 @@ def all_subclasses(cls: Any) -> List[Any]:
]
-def add_logging_option_arguments(parser):
+def add_logging_level_option_arguments(parser):
"""Add options to an argument parser to configure logging levels."""
# arguments for logging configuration


message: adding module name to log formatter

diff:
@@ -106,7 +106,7 @@ def configure_file_logging(
if not log_file:
return
- formatter = logging.Formatter("%(asctime)s [%(levelname)-5.5s] %(message)s")
+ formatter = logging.Formatter("%(asctime)s [%(levelname)-5.5s] %(name)s - %(message)s")
file_handler = logging.FileHandler(log_file, encoding="utf-8")
file_handler.setLevel(logger_obj.level)
file_handler.setFormatter(formatter)

original_message: adding module name to log formatter


message: added application root logger name

diff:
@@ -3,3 +3,4 @@ DEFAULT_SANIC_WORKERS = 1
ENV_SANIC_WORKERS = "ACTION_SERVER_SANIC_WORKERS"
DEFAULT_LOG_LEVEL_LIBRARIES = "ERROR"
ENV_LOG_LEVEL_LIBRARIES = "LOG_LEVEL_LIBRARIES"
+APPLICATION_ROOT_LOGGER_NAME = "rasa_sdk"

original_message: added application root logger name


message: logging to file configuration changed from root logger to application root logger

diff:
@@ -2,6 +2,7 @@ import logging
from rasa_sdk import utils
from rasa_sdk.endpoint import create_argument_parser, run
+from rasa_sdk.constants import APPLICATION_ROOT_LOGGER_NAME
def main_from_args(args):
@@ -9,7 +10,7 @@ def main_from_args(args):
logging.getLogger("matplotlib").setLevel(logging.WARN)
utils.configure_colored_logging(args.loglevel)
- utils.configure_file_logging(logging.root, args.log_file)
+ utils.configure_file_logging(logging.getLogger(APPLICATION_ROOT_LOGGER_NAME), args.log_file)
utils.update_sanic_log_level()
run(


message: formatting code in utils.py

diff:
@@ -94,9 +94,7 @@ def configure_colored_logging(loglevel):
)
-def configure_file_logging(
- logger_obj: logging.Logger, log_file: Optional[Text]
-) -> None:
+def configure_file_logging(logger_obj: logging.Logger, log_file: Optional[Text]) -> None:
"""Configure logging to a file.
Args:

original_message: formatting code in utils.py


message: logging level of root logger set to args.loglevel to configure from cmd line

diff:
@@ -6,11 +6,10 @@ from rasa_sdk.constants import APPLICATION_ROOT_LOGGER_NAME
def main_from_args(args):
- logging.basicConfig(level=logging.DEBUG)
logging.getLogger("matplotlib").setLevel(logging.WARN)
utils.configure_colored_logging(args.loglevel)
- utils.configure_file_logging(logging.getLogger(APPLICATION_ROOT_LOGGER_NAME), args.log_file)
+ utils.configure_file_logging(logging.getLogger(APPLICATION_ROOT_LOGGER_NAME), args.log_file, args.loglevel)
utils.update_sanic_log_level()
run(

diff:
@@ -94,19 +94,23 @@ def configure_colored_logging(loglevel):
)
-def configure_file_logging(logger_obj: logging.Logger, log_file: Optional[Text]) -> None:
+def configure_file_logging(logger_obj: logging.Logger, log_file: Optional[Text], loglevel: int) -> None:
"""Configure logging to a file.
Args:
- logger_obj: Logger object to configure.
- log_file: Path of log file to write to.
+ :param logger_obj: Logger object to configure.
+ :param log_file: Path of log file to write to.
+ :param loglevel:
"""
if not log_file:
return
+ if not loglevel:
+ loglevel = logging.INFO
+
formatter = logging.Formatter("%(asctime)s [%(levelname)-5.5s] %(name)s - %(message)s")
file_handler = logging.FileHandler(log_file, encoding="utf-8")
- file_handler.setLevel(logger_obj.level)
+ file_handler.setLevel(loglevel)
file_handler.setFormatter(formatter)
logger_obj.addHandler(file_handler)


message: fixing code formatting

diff:
@@ -9,7 +9,9 @@ def main_from_args(args):
logging.getLogger("matplotlib").setLevel(logging.WARN)
utils.configure_colored_logging(args.loglevel)
- utils.configure_file_logging(logging.getLogger(APPLICATION_ROOT_LOGGER_NAME), args.log_file, args.loglevel)
+ utils.configure_file_logging(
+ logging.getLogger(APPLICATION_ROOT_LOGGER_NAME), args.log_file, args.loglevel
+ )
utils.update_sanic_log_level()
run(

diff:
@@ -95,7 +95,9 @@ def configure_colored_logging(loglevel):
)
-def configure_file_logging(logger_obj: logging.Logger, log_file: Optional[Text], loglevel: int) -> None:
+def configure_file_logging(
+ logger_obj: logging.Logger, log_file: Optional[Text], loglevel: int
+) -> None:
"""Configure logging to a file.
Args:
@@ -109,7 +111,9 @@ def configure_file_logging(logger_obj: logging.Logger, log_file: Optional[Text],
if not loglevel:
loglevel = logging.INFO
- formatter = logging.Formatter("%(asctime)s [%(levelname)-5.5s] %(name)s - %(message)s")
+ formatter = logging.Formatter(
+ "%(asctime)s [%(levelname)-5.5s] %(name)s - %(message)s"
+ )
file_handler = logging.FileHandler(log_file, encoding="utf-8")
file_handler.setLevel(loglevel)
file_handler.setFormatter(formatter)

original_message: fixing code formatting


message:
fixing code lintings for rasa_sdk/__main__.py
rasa_sdk/utils.py

diff:
@@ -6,6 +6,7 @@ from rasa_sdk.constants import APPLICATION_ROOT_LOGGER_NAME
def main_from_args(args):
+ """Run with arguments."""
logging.getLogger("matplotlib").setLevel(logging.WARN)
utils.configure_colored_logging(args.loglevel)

diff:
@@ -42,7 +42,6 @@ def all_subclasses(cls: Any) -> List[Any]:
def add_logging_level_option_arguments(parser):
"""Add options to an argument parser to configure logging levels."""
-
# arguments for logging configuration
parser.add_argument(
"-v",
@@ -71,6 +70,7 @@ def add_logging_level_option_arguments(parser):
def add_logging_file_arguments(parser):
+ """Add options to an argument parser to configure logging to a file."""
parser.add_argument(
"--log-file",
type=str,
@@ -80,6 +80,7 @@ def add_logging_file_arguments(parser):
def configure_colored_logging(loglevel):
+ """Configure logging with colors."""
import coloredlogs
field_styles = coloredlogs.DEFAULT_FIELD_STYLES.copy()
@@ -100,10 +101,10 @@ def configure_file_logging(
) -> None:
"""Configure logging to a file.
- Args:
:param logger_obj: Logger object to configure.
:param log_file: Path of log file to write to.
- :param loglevel:
+ :param loglevel: Log Level
+ :return:
"""
if not log_file:
return
@@ -122,7 +123,6 @@ def configure_file_logging(
def arguments_of(func) -> AbstractSet[Text]:
"""Return the parameters of the function `func` as a list of their names."""
-
return inspect.signature(func).parameters.keys()
@@ -225,6 +225,7 @@ def update_sanic_log_level() -> None:
async def call_potential_coroutine(
coroutine_or_return_value: Union[Any, Coroutine]
) -> Any:
+ """Await if its a coroutine."""
if asyncio.iscoroutine(coroutine_or_return_value):
return await coroutine_or_return_value


message: fixing code linting errors for rasa_sdk/interfaces.py

diff:
@@ -153,8 +153,8 @@ class Tracker:
def idx_after_latest_restart(self) -> int:
"""Return the idx of the most recent restart in the list of events.
- If the conversation has not been restarted, `0` is returned."""
-
+ If the conversation has not been restarted, `0` is returned.
+ """
idx = 0
for i, event in enumerate(self.events):
if event.get("event") == "restart":


message: bitwarden: fix clipboard copy error that may include newline

diff:
@@ -54,7 +54,7 @@ if [[ -z $name || -z $password ]]; then
exit 1
fi
-echo $password | pbcopy
+echo $password | tr -d " \t\n\r" | pbcopy
unset $password
echo "Copied the password for '$name' to the clipboard."
exit 0

license: MIT License

repo: raycast/script-commands


message: bitwarden: fix message when trying to copy password for an unexisting account

diff:
@@ -49,7 +49,7 @@ item=$(bw list items --search $1 $session 2> /dev/null | jq ".[0] | { name: .nam
name=$(echo $item | jq ".name")
password=$(echo $item | jq --raw-output ".password")
-if [[ -z $name || -z $password ]]; then
+if [[ -z $name || -z $password || ( $name == "null" && $password == "null") ]]; then
echo "The query '$1' did not return a password."
exit 1
fi


message: bitwarden: cleanest way to avoid newline at the end of password

diff:
@@ -54,7 +54,7 @@ if [[ -z $name || -z $password || ( $name == "null" && $password == "null") ]];
exit 1
fi
-echo $password | tr -d " \t\n\r" | pbcopy
unset $password
+echo -n $password | pbcopy
echo "Copied the password for '$name' to the clipboard."
exit 0


message: bitwarden: cleanest way to handle non existing password

diff:
# @raycast.author Phil Salant
# @raycast.authorURL https://github.com/PSalant726
# @raycast.description Search all items in a Bitwarden vault, and copy the password of the first search result to the clipboard.
+notFound() {
+ echo -n "The query '$1' did not return a password."
+ exit 1
+}
if ! command -v bw &> /dev/null; then
echo "The Bitwarden CLI is not installed."
@@ -46,13 +50,8 @@ if [ $unlocked_status -ne 0 ]; then
fi
item=$(bw list items --search $1 $session 2> /dev/null | jq ".[0] | { name: .name, password: .login.password }")
-name=$(echo $item | jq ".name")
-password=$(echo $item | jq --raw-output ".password")
-
-if [[ -z $name || -z $password || ( $name == "null" && $password == "null") ]]; then
- echo "The query '$1' did not return a password."
- exit 1
-fi
+name=$(echo $item | jq -e ".name") || notFoundError
+password=$(echo $item | jq -re ".password") || notFoundError
unset $password
echo -n $password | pbcopy


message: bitwarden: fixed unsetting variables

diff:
@@ -53,7 +53,8 @@ item=$(bw list items --search $1 $session 2> /dev/null | jq ".[0] | { name: .nam
name=$(echo $item | jq -e ".name") || notFoundError
password=$(echo $item | jq -re ".password") || notFoundError
-unset $password
echo -n $password | pbcopy
+unset password
echo "Copied the password for '$name' to the clipboard."
+unset name
exit 0
\ No newline at end of file

original_message: bitwarden: fixed unsetting variables


diff:
@@ -50,8 +50,8 @@ if [ $unlocked_status -ne 0 ]; then
fi
item=$(bw list items --search $1 $session 2> /dev/null | jq ".[0] | { name: .name, password: .login.password }")
-name=$(echo $item | jq -e ".name") || notFoundError
-password=$(echo $item | jq -re ".password") || notFoundError
+name=$(echo $item | jq --exit-status ".name") || notFoundError
+password=$(echo $item | jq --raw-output --exit-status ".password") || notFoundError
echo -n $password | pbcopy
unset password

original_message:
bitwarden: changed jq params to be human readable

Co-authored-by: Phil Salant <PSalant726@users.noreply.github.com>


message: bitwarden: fixed notFound function

diff:
# @raycast.authorURL https://github.com/PSalant726
# @raycast.description Search all items in a Bitwarden vault, and copy the password of the first search result to the clipboard.
notFound() {
- echo -n "The query '$1' did not return a password."
+ echo "The query '$1' did not return a password."
exit 1
}

original_message:
bitwarden: fixed notFound function

Co-authored-by: Phil Salant <PSalant726@users.noreply.github.com>


message: bitwarden: formatting

diff:
# @raycast.author Phil Salant
# @raycast.authorURL https://github.com/PSalant726
# @raycast.description Search all items in a Bitwarden vault, and copy the password of the first search result to the clipboard.
+
notFound() {
echo "The query '$1' did not return a password."
exit 1
@@ -56,5 +57,4 @@ password=$(echo $item | jq --raw-output --exit-status ".password") || notFoundEr
echo -n $password | pbcopy
unset password
echo "Copied the password for '$name' to the clipboard."
-unset name
exit 0

original_message: bitwarden: formatting


diff:
# Required parameters:
# @raycast.schemaVersion 1
# @raycast.title Update Community Scripts
-# @raycast.description Updates community scripts to their last available version from the Github repository.
+# @raycast.description Updates community Script Commands to their last available version from the GitHub repository.
# @raycast.mode compact
# @raycast.refreshTime 1d

original_message:
Update commands/system/update-scripts-command.template.py

Co-authored-by: Thiago Holanda <unnamedd@gmail.com>


message: Fixed clean docker script

diff:
#!/bin/bash
+ # Dependency: This script requires `docker for mac` to be installed: https://docs.docker.com/docker-for-mac/install/
+
# Required parameters:
# @raycast.schemaVersion 1
-# @raycast.title Clean Docker
+# @raycast.title Clean
# @raycast.mode compact
# @raycast.packageName Docker
# Documentation:
# @raycast.description Script that cleans Docker images, volumes, and containers
+# @raycast.author Quentin Eude
+# @raycase.authorURL https://github.com/qeude
+
+if ! command -v docker &> /dev/null; then
+ echo "docker for mac is required (https://docs.docker.com/docker-for-mac/install/).";
+ exit 1;
+fi
docker stop $(docker ps -a -q) && docker rm $(docker ps -a -q)
docker image prune -a -f

original_message: Fixed clean docker script


message:
Adding Traits cache instead of recursion.
Removed recursion in AddTraitsFromTestNode method.
Cached the class and assembly level traits so that we don't need to traverse the tree again.
Reading test-case, test-suite and assembly level attributes based on the tree structure.

diff:
@@ -158,7 +158,7 @@ namespace NUnit.VisualStudio.TestAdapter
private int ProcessTestCases(XmlNode topNode, ITestCaseDiscoverySink discoverySink, TestConverter testConverter)
{
int cases = 0;
-
+ testConverter.BuildAttributesCache(topNode);
foreach (XmlNode testNode in topNode.SelectNodes("//test-case"))
{
try

diff:
@@ -39,6 +39,7 @@ namespace NUnit.VisualStudio.TestAdapter
private readonly string _sourceAssembly;
private NavigationDataProvider _navigationDataProvider;
private bool _collectSourceInformation;
+ private IDictionary<string, List<KeyValuePair<string, string>>> _attributesCache;
#region Constructor
@@ -118,10 +119,60 @@ namespace NUnit.VisualStudio.TestAdapter
return results;
}
+ /// <summary>
+ /// Builds attributes cache for the testrun / assembly
+ /// </summary>
+ /// <param name="topNode"></param>
+ public void BuildAttributesCache(XmlNode topNode)
+ {
+ var assemblyAttributes = topNode.SelectNodes("properties/property");
+ this.ReadAttributes(assemblyAttributes);
+
+ var testSuiteAttributes = topNode.SelectNodes("test-suite/test-suite/properties/property");
+ this.ReadAttributes(testSuiteAttributes);
+
+ var testCaseAttributes = topNode.SelectNodes("test-suite/test-suite/test-case/properties/property");
+ this.ReadAttributes(testCaseAttributes);
+
+ }
+
#endregion
#region Helper Methods
+ private void ReadAttributes(XmlNodeList attributes)
+ {
+ var enumerator = attributes.GetEnumerator();
+
+ while(enumerator.MoveNext())
+ {
+ var property = (XmlNode)enumerator.Current;
+ var parentId = property.ParentNode?.ParentNode?.Attributes["id"]?.Value ?? "0-0000";
+
+ var propertyName = property.Attributes["name"]?.Value;
+ var propertyValue = property.Attributes["value"]?.Value;
+
+ this.AddAttributesToCache(parentId, propertyName, propertyValue);
+ }
+ }
+
+ private void AddAttributesToCache(string parentId, string propertyName, string propertyValue)
+ {
+ if (!string.IsNullOrEmpty(propertyName) && propertyName[0] != '_' && !string.IsNullOrEmpty(propertyValue))
+ {
+ if (_attributesCache.ContainsKey(parentId))
+ {
+ _attributesCache[parentId].Add(new KeyValuePair<string, string>(propertyName, propertyValue));
+ }
+ else
+ {
+ var kps = new List<KeyValuePair<string, string>>();
+ kps.Add(new KeyValuePair<string, string>(propertyName, propertyValue));
+ _attributesCache[parentId] = kps;
+ }
+ }
+ }
+
/// <summary>
/// Makes a TestCase from an NUnit test, adding
/// navigation data if it can be found.
@@ -150,7 +201,7 @@ namespace NUnit.VisualStudio.TestAdapter
}
}
- testCase.AddTraitsFromTestNode(testNode);
+ testCase.AddTraitsFromTestNode(testNode, this._attributesCache);
return testCase;
}

diff:
@@ -68,10 +68,33 @@ namespace NUnit.VisualStudio.TestAdapter
}
}
- public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode)
+ public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, IDictionary<string,List<KeyValuePair<string,string>>> propertiesCache)
{
if (IsSupported)
- AddTraitsFromTestNode(testNode, TraitsProperty.GetValue(testCase, new object[0]));
+ {
+ var assemblyId = testNode.ParentNode?.ParentNode?.Attributes["id"]?.Value;
+ List<KeyValuePair<string, string>> testCaseProperties = new List<KeyValuePair<string, string>>();
+
+ // Reading properties at the assembly level.
+ if(!string.IsNullOrEmpty(assemblyId) && propertiesCache.ContainsKey(assemblyId))
+ testCaseProperties = propertiesCache[assemblyId];
+
+ // Reading properties at the class (test suite) level.
+ var testSuiteId = testNode.ParentNode?.Attributes["id"]?.Value;
+ if(!string.IsNullOrEmpty(testSuiteId) && propertiesCache.ContainsKey(testSuiteId))
+ testCaseProperties.AddRange(propertiesCache[testSuiteId]);
+
+ // Reading properties at the test case level.
+ var testCaseId = testNode?.Attributes["id"].Value;
+ if(!string.IsNullOrEmpty(testCaseId) && propertiesCache.ContainsKey(testCaseId))
+ testCaseProperties.AddRange(propertiesCache[testCaseId]);
+
+ var obj = TraitsProperty.GetValue(testCase, new object[0]);
+ foreach (var property in testCaseProperties)
+ {
+ TraitsCollectionAdd.Invoke(obj, new object[] { property.Key, property.Value });
+ }
+ }
}
private static void AddTraitsFromTestNode(XmlNode test, object traitsCollection)

original_message:
Adding Traits cache instead of recursion.

- Removed recursion in AddTraitsFromTestNode method.
- Cached the class and assembly level traits so that we don't need to traverse the tree again.
- Reading test-case, test-suite and assembly level attributes based on the tree structure.


message:
Building the cache as we go, so that it will not break the compat.
Added a test to check the cache built.

diff:
@@ -158,7 +158,6 @@ namespace NUnit.VisualStudio.TestAdapter
private int ProcessTestCases(XmlNode topNode, ITestCaseDiscoverySink discoverySink, TestConverter testConverter)
{
int cases = 0;
- testConverter.BuildAttributesCache(topNode);
foreach (XmlNode testNode in topNode.SelectNodes("//test-case"))
{
try

diff:
@@ -49,6 +49,7 @@ namespace NUnit.VisualStudio.TestAdapter
_sourceAssembly = sourceAssembly;
_vsTestCaseMap = new Dictionary<string, TestCase>();
_collectSourceInformation = collectSourceInformation;
+ _attributesCache = new Dictionary<string, List<KeyValuePair<string, string>>>();
if (_collectSourceInformation)
{
@@ -58,8 +59,15 @@ namespace NUnit.VisualStudio.TestAdapter
#endregion
- #region Public Methods
+ public IDictionary<string, List<KeyValuePair<string, string>>> AttributesCache
+ {
+ get
+ {
+ return _attributesCache;
+ }
+ }
+ #region Public Methods
/// <summary>
/// Converts an NUnit test into a TestCase for Visual Studio,
/// using the best method available according to the exact
@@ -118,61 +126,9 @@ namespace NUnit.VisualStudio.TestAdapter
return results;
}
-
- /// <summary>
- /// Builds attributes cache for the testrun / assembly
- /// </summary>
- /// <param name="topNode"></param>
- public void BuildAttributesCache(XmlNode topNode)
- {
- var assemblyAttributes = topNode.SelectNodes("properties/property");
- this.ReadAttributes(assemblyAttributes);
-
- var testSuiteAttributes = topNode.SelectNodes("test-suite/test-suite/properties/property");
- this.ReadAttributes(testSuiteAttributes);
-
- var testCaseAttributes = topNode.SelectNodes("test-suite/test-suite/test-case/properties/property");
- this.ReadAttributes(testCaseAttributes);
-
- }
-
#endregion
#region Helper Methods
-
- private void ReadAttributes(XmlNodeList attributes)
- {
- var enumerator = attributes.GetEnumerator();
-
- while(enumerator.MoveNext())
- {
- var property = (XmlNode)enumerator.Current;
- var parentId = property.ParentNode?.ParentNode?.Attributes["id"]?.Value ?? "0-0000";
-
- var propertyName = property.Attributes["name"]?.Value;
- var propertyValue = property.Attributes["value"]?.Value;
-
- this.AddAttributesToCache(parentId, propertyName, propertyValue);
- }
- }
-
- private void AddAttributesToCache(string parentId, string propertyName, string propertyValue)
- {
- if (!string.IsNullOrEmpty(propertyName) && propertyName[0] != '_' && !string.IsNullOrEmpty(propertyValue))
- {
- if (_attributesCache.ContainsKey(parentId))
- {
- _attributesCache[parentId].Add(new KeyValuePair<string, string>(propertyName, propertyValue));
- }
- else
- {
- var kps = new List<KeyValuePair<string, string>>();
- kps.Add(new KeyValuePair<string, string>(propertyName, propertyValue));
- _attributesCache[parentId] = kps;
- }
- }
- }
-
/// <summary>
/// Makes a TestCase from an NUnit test, adding
/// navigation data if it can be found.

diff:
@@ -39,6 +39,7 @@ namespace NUnit.VisualStudio.TestAdapter
static TraitsFeature()
{
+ System.Diagnostics.Debugger.Launch();
TraitsProperty = typeof(TestCase).GetProperty("Traits");
if (TraitsProperty != null)
{
@@ -72,49 +73,85 @@ namespace NUnit.VisualStudio.TestAdapter
{
if (IsSupported)
{
- var assemblyId = testNode.ParentNode?.ParentNode?.Attributes["id"]?.Value;
- List<KeyValuePair<string, string>> testCaseProperties = new List<KeyValuePair<string, string>>();
-
- // Reading properties at the assembly level.
- if(!string.IsNullOrEmpty(assemblyId) && propertiesCache.ContainsKey(assemblyId))
- testCaseProperties = propertiesCache[assemblyId];
+ AddTraitsFromTestNode(testNode, TraitsProperty.GetValue(testCase, new object[0]), propertiesCache);
+ }
+ }
- // Reading properties at the class (test suite) level.
- var testSuiteId = testNode.ParentNode?.Attributes["id"]?.Value;
- if(!string.IsNullOrEmpty(testSuiteId) && propertiesCache.ContainsKey(testSuiteId))
- testCaseProperties.AddRange(propertiesCache[testSuiteId]);
+ private static bool IsInternalProperty(string propertyName, string propertyValue)
+ {
+ // Property names starting with '_' are for internal use only
+ return string.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || string.IsNullOrEmpty(propertyValue);
+ }
- // Reading properties at the test case level.
- var testCaseId = testNode?.Attributes["id"].Value;
- if(!string.IsNullOrEmpty(testCaseId) && propertiesCache.ContainsKey(testCaseId))
- testCaseProperties.AddRange(propertiesCache[testCaseId]);
+ private static void AddTraitsFromTestNode(XmlNode test, object traitsCollection, IDictionary<string, List<KeyValuePair<string, string>>> propertiesCache)
+ {
+ var ancestor = test.ParentNode;
+ var key = ancestor.Attributes?["id"]?.Value;
- var obj = TraitsProperty.GetValue(testCase, new object[0]);
- foreach (var property in testCaseProperties)
+ // Reading ancestor properties of a test-case node. And cacheing it.
+ while (ancestor != null && key != null)
{
- TraitsCollectionAdd.Invoke(obj, new object[] { property.Key, property.Value });
- }
+ if(propertiesCache.ContainsKey(key))
+ {
+ foreach (var property in propertiesCache[key])
+ {
+ TraitsCollectionAdd.Invoke(traitsCollection, new object[] { property.Key, property.Value });
}
}
+ else
+ {
+ var nodesList = ancestor.SelectNodes("properties/property");
+ foreach (XmlNode propertyNode in nodesList)
+ {
+ string propertyName = propertyNode.GetAttribute("name");
+ string propertyValue = propertyNode.GetAttribute("value");
- private static void AddTraitsFromTestNode(XmlNode test, object traitsCollection)
+ AddAttributesToCache(propertiesCache, key, propertyName, propertyValue);
+ if (!IsInternalProperty(propertyName, propertyValue))
{
- if (test.ParentNode != null)
- AddTraitsFromTestNode(test.ParentNode, traitsCollection);
+ TraitsCollectionAdd.Invoke(traitsCollection, new object[] { propertyName, propertyValue });
+ }
+ }
+ // Adding empty list to dictionary, so that we will not make SelectNodes call again.
+ if(nodesList.Count == 0 && !propertiesCache.ContainsKey(key))
+ {
+ propertiesCache[key] = new List<KeyValuePair<string, string>>();
+ }
+ }
+ ancestor = ancestor.ParentNode;
+ key = ancestor?.Attributes?["id"]?.Value;
+ }
+ // No Need to store test-case properties in dictionary.
foreach (XmlNode propertyNode in test.SelectNodes("properties/property"))
{
string propertyName = propertyNode.GetAttribute("name");
string propertyValue = propertyNode.GetAttribute("value");
- // Property names starting with '_' are for internal use only
- if (!string.IsNullOrEmpty(propertyName) && propertyName[0] != '_' && !string.IsNullOrEmpty(propertyValue))
+ if (!IsInternalProperty(propertyName, propertyValue))
{
TraitsCollectionAdd.Invoke(traitsCollection, new object[] { propertyName, propertyValue });
}
}
}
+ private static void AddAttributesToCache(IDictionary<string, List<KeyValuePair<string, string>>> propertiesCache, string key, string propertyName, string propertyValue)
+ {
+ if (propertiesCache.ContainsKey(key) && !IsInternalProperty(propertyName, propertyValue))
+ {
+ propertiesCache[key].Add(new KeyValuePair<string, string>(propertyName, propertyValue));
+ }
+ else
+ {
+ var kps = new List<KeyValuePair<string, string>>();
+ if (!IsInternalProperty(propertyName, propertyValue))
+ {
+ kps.Add(new KeyValuePair<string, string>(propertyName, propertyValue));
+ }
+ propertiesCache[key] = kps;
+ }
+ }
+
public static IEnumerable<NTrait> GetTraits(this TestCase testCase)
{
var traits = new List<NTrait>();

diff:
@@ -52,8 +52,20 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
public void CanMakeTestCaseFromTest()
{
var testCase = testConverter.ConvertTestCase(fakeTestNode);
+ CheckTestCase(testCase);
+ }
+
+ [Test]
+ public void CanMakeTestCaseFromTestWithCache()
+ {
+
+ var testCase = testConverter.ConvertTestCase(fakeTestNode);
CheckTestCase(testCase);
+ if(TraitsFeature.IsSupported)
+ {
+ Assert.That(testConverter.AttributesCache.Keys.Count, Is.EqualTo(2));
+ }
}
[Test]


message:
Removed the check to verify if traits are supported by reflection method.
Thus this will not work with older object models.

diff:
<add key="AppVeyor NUnit CI Feed" value="https://ci.appveyor.com/nuget/nunit" />
<add key="AppVeyor NUnit Engine CI Feed" value="https://ci.appveyor.com/nuget/nunit-console" />
<add key="NUnit MyGet Feed" value="https://www.myget.org/F/nunit/api/v2" />
+ <add key="api.nuget.org" value="https://api.nuget.org/v3/index.json" />
</packageSources>
</configuration>

diff:
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net35'">
- <PackageReference Include="Microsoft.VisualStudio.TestPlatform.ObjectModel" Version="0.0.3" PrivateAssets="All" />
+ <PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.0" PrivateAssets="All" />
<PackageReference Include="Mono.Cecil" Version="0.9.6.4" />
<PackageReference Include="nunit.engine" Version="3.7.0" PrivateAssets="All" />
</ItemGroup>

diff:
@@ -39,7 +39,7 @@ namespace NUnit.VisualStudio.TestAdapter
private readonly string _sourceAssembly;
private NavigationDataProvider _navigationDataProvider;
private bool _collectSourceInformation;
- private IDictionary<string, List<KeyValuePair<string, string>>> _attributesCache;
+ private IDictionary<string, List<Trait>> _attributesCache;
#region Constructor
@@ -49,7 +49,7 @@ namespace NUnit.VisualStudio.TestAdapter
_sourceAssembly = sourceAssembly;
_vsTestCaseMap = new Dictionary<string, TestCase>();
_collectSourceInformation = collectSourceInformation;
- _attributesCache = new Dictionary<string, List<KeyValuePair<string, string>>>();
+ _attributesCache = new Dictionary<string, List<Trait>>();
if (_collectSourceInformation)
{
@@ -59,7 +59,7 @@ namespace NUnit.VisualStudio.TestAdapter
#endregion
- public IDictionary<string, List<KeyValuePair<string, string>>> AttributesCache
+ public IDictionary<string, List<Trait>> AttributesCache
{
get
{

diff:
@@ -32,60 +32,17 @@ namespace NUnit.VisualStudio.TestAdapter
{
public static class TraitsFeature
{
- private static readonly PropertyInfo TraitsProperty;
- private static readonly MethodInfo TraitsCollectionAdd;
- private static readonly PropertyInfo NameProperty;
- private static readonly PropertyInfo ValueProperty;
-
- static TraitsFeature()
- {
- System.Diagnostics.Debugger.Launch();
- TraitsProperty = typeof(TestCase).GetProperty("Traits");
- if (TraitsProperty != null)
- {
-
- var traitsCollectionType = TraitsProperty.PropertyType;
- TraitsCollectionAdd = traitsCollectionType.GetMethod("Add", new[] { typeof(string), typeof(string) });
-
- var traitType = Type.GetType("Microsoft.VisualStudio.TestPlatform.ObjectModel.Trait,Microsoft.VisualStudio.TestPlatform.ObjectModel");
- if (traitType != null)
- {
- NameProperty = traitType.GetProperty("Name");
- ValueProperty = traitType.GetProperty("Value");
- }
- }
-
- IsSupported = TraitsProperty != null && NameProperty != null && ValueProperty != null;
- }
-
- public static bool IsSupported { get; private set; }
-
public static void AddTrait(this TestCase testCase, string name, string value)
{
- if (TraitsCollectionAdd != null)
+ if(testCase != null)
{
- object traitsCollection = TraitsProperty.GetValue(testCase, new object[0]);
- TraitsCollectionAdd.Invoke(traitsCollection, new object[] { name, value });
+ testCase.Traits.Add(new Trait(name, value));
}
}
- public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, IDictionary<string,List<KeyValuePair<string,string>>> propertiesCache)
- {
- if (IsSupported)
+ public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, IDictionary<string,List<Trait>> propertiesCache)
{
- AddTraitsFromTestNode(testNode, TraitsProperty.GetValue(testCase, new object[0]), propertiesCache);
- }
- }
-
- private static bool IsInternalProperty(string propertyName, string propertyValue)
- {
- // Property names starting with '_' are for internal use only
- return string.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || string.IsNullOrEmpty(propertyValue);
- }
-
- private static void AddTraitsFromTestNode(XmlNode test, object traitsCollection, IDictionary<string, List<KeyValuePair<string, string>>> propertiesCache)
- {
- var ancestor = test.ParentNode;
+ var ancestor = testNode.ParentNode;
var key = ancestor.Attributes?["id"]?.Value;
// Reading ancestor properties of a test-case node. And cacheing it.
@@ -93,10 +50,7 @@ namespace NUnit.VisualStudio.TestAdapter
{
if (propertiesCache.ContainsKey(key))
{
- foreach (var property in propertiesCache[key])
- {
- TraitsCollectionAdd.Invoke(traitsCollection, new object[] { property.Key, property.Value });
- }
+ testCase.Traits.AddRange(propertiesCache[key]);
}
else
{
@@ -109,13 +63,13 @@ namespace NUnit.VisualStudio.TestAdapter
AddAttributesToCache(propertiesCache, key, propertyName, propertyValue);
if (!IsInternalProperty(propertyName, propertyValue))
{
- TraitsCollectionAdd.Invoke(traitsCollection, new object[] { propertyName, propertyValue });
+ testCase.Traits.Add(new Trait(propertyName, propertyValue));
}
}
// Adding empty list to dictionary, so that we will not make SelectNodes call again.
if (nodesList.Count == 0 && !propertiesCache.ContainsKey(key))
{
- propertiesCache[key] = new List<KeyValuePair<string, string>>();
+ propertiesCache[key] = new List<Trait>();
}
}
ancestor = ancestor.ParentNode;
@@ -123,32 +77,38 @@ namespace NUnit.VisualStudio.TestAdapter
}
// No Need to store test-case properties in dictionary.
- foreach (XmlNode propertyNode in test.SelectNodes("properties/property"))
+ foreach (XmlNode propertyNode in testNode.SelectNodes("properties/property"))
{
string propertyName = propertyNode.GetAttribute("name");
string propertyValue = propertyNode.GetAttribute("value");
if (!IsInternalProperty(propertyName, propertyValue))
{
- TraitsCollectionAdd.Invoke(traitsCollection, new object[] { propertyName, propertyValue });
+ testCase.Traits.Add(new Trait(propertyName, propertyValue));
}
}
}
- private static void AddAttributesToCache(IDictionary<string, List<KeyValuePair<string, string>>> propertiesCache, string key, string propertyName, string propertyValue)
+ private static bool IsInternalProperty(string propertyName, string propertyValue)
+ {
+ // Property names starting with '_' are for internal use only
+ return string.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || string.IsNullOrEmpty(propertyValue);
+ }
+
+ private static void AddAttributesToCache(IDictionary<string, List<Trait>> propertiesCache, string key, string propertyName, string propertyValue)
{
if (propertiesCache.ContainsKey(key) && !IsInternalProperty(propertyName, propertyValue))
{
- propertiesCache[key].Add(new KeyValuePair<string, string>(propertyName, propertyValue));
+ propertiesCache[key].Add(new Trait(propertyName, propertyValue));
}
else
{
- var kps = new List<KeyValuePair<string, string>>();
+ var traits = new List<Trait>();
if (!IsInternalProperty(propertyName, propertyValue))
{
- kps.Add(new KeyValuePair<string, string>(propertyName, propertyValue));
+ traits.Add(new Trait(propertyName, propertyValue));
}
- propertiesCache[key] = kps;
+ propertiesCache[key] = traits;
}
}
@@ -156,14 +116,10 @@ namespace NUnit.VisualStudio.TestAdapter
{
var traits = new List<NTrait>();
- if (IsSupported)
+ if (testCase != null && testCase.Traits != null)
{
- var traitsCollection = TraitsProperty.GetValue(testCase, new object[0]) as IEnumerable<object>;
-
- if (traitsCollection != null)
- traits.AddRange(from traitObject in traitsCollection let name = NameProperty.GetValue(traitObject, new object[0]) as string let value = ValueProperty.GetValue(traitObject, new object[0]) as string select new NTrait(name, value));
+ traits.AddRange(from trait in testCase.Traits let name = trait.Name let value = trait.Value select new NTrait(name, value));
}
-
return traits;
}
}

diff:
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'net45' ">
- <PackageReference Include="Microsoft.VisualStudio.TestPlatform.ObjectModel" Version="0.0.3" PrivateAssets="All" />
+ <PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.0" PrivateAssets="All" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">

diff:
@@ -62,10 +62,12 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
var testCase = testConverter.ConvertTestCase(fakeTestNode);
CheckTestCase(testCase);
- if(TraitsFeature.IsSupported)
- {
- Assert.That(testConverter.AttributesCache.Keys.Count, Is.EqualTo(2));
- }
+
+ Assert.That(testConverter.AttributesCache.Keys.Count, Is.EqualTo(1));
+ Assert.That(testConverter.AttributesCache["121"].Count, Is.EqualTo(1));
+ var parentTrait = testConverter.AttributesCache["121"];
+ Assert.That(parentTrait[0].Name, Is.EqualTo("Category"));
+ Assert.That(parentTrait[0].Value, Is.EqualTo("super"));
}
[Test]
@@ -116,14 +118,8 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.That(testCase.LineNumber, Is.EqualTo(FakeTestData.LineNumber));
}
- // Check traits using reflection, since the feature was added
- // in an update to VisualStudio and may not be present.
- if (TraitsFeature.IsSupported)
- {
var traitList = testCase.GetTraits().Select(trait => trait.Name + ":" + trait.Value).ToList();
-
Assert.That(traitList, Is.EquivalentTo(new[] { "Category:super", "Category:cat1", "Priority:medium" }));
}
}
}
-}

diff:
@@ -45,8 +45,6 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
[Test]
public void PropertyValueProviderCategoryWithOneCategory()
{
- CheckTraitsSupported();
-
var tc = new TestCase("Test1", new Uri("executor://NUnitTestExecutor"), "NUnit.VSIX");
tc.AddTrait("Category", "CI");
var testFilter = new TfsTestFilter(null);
@@ -57,8 +55,6 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
[Test]
public void PropertyValueProviderCategoryWithNoTraits()
{
- CheckTraitsSupported();
-
var tc = new TestCase("Test1", new Uri("executor://NUnitTestExecutor"), "NUnit.VSIX");
var testFilter = new TfsTestFilter(null);
var obj = TfsTestFilter.PropertyValueProvider(tc, "TestCategory");
@@ -68,8 +64,6 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
[Test]
public void PropertyValueProviderCategoryWithMultipleCategories()
{
- CheckTraitsSupported();
-
var tc = new TestCase("Test1", new Uri("executor://NUnitTestExecutor"), "NUnit.VSIX");
tc.AddTrait("Category", "CI");
tc.AddTrait("Category", "MyOwn");
@@ -90,11 +84,5 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
var obj = TfsTestFilter.PropertyValueProvider(tc, "Garbage");
Assert.Null(obj);
}
-
- private static void CheckTraitsSupported()
- {
- if (!TraitsFeature.IsSupported)
- Assert.Inconclusive("This version of Visual Studio does not support Traits");
- }
}
}


message: Added one more test to cover the diverse XML.

diff:
@@ -34,7 +34,7 @@ namespace NUnit.VisualStudio.TestAdapter.Tests.Fakes
{
// ReSharper disable once UnusedMember.Local
private static void FakeTestCase() { } // LineNumber should be this line
-
+ #region TestXmls
public const string TestXml =
@"<test-suite
id='121'
@@ -57,6 +57,64 @@ namespace NUnit.VisualStudio.TestAdapter.Tests.Fakes
</test-case>
</test-suite>";
+ public const string TestXml2 = @"<test-run id='2' name='nUnitClassLibrary.dll' fullname='C:\Users\navb\source\repos\nUnitClassLibrary\nUnitClassLibrary\bin\Debug\nUnitClassLibrary.dll' testcasecount='5'>
+ <test-suite type='Assembly' id='0-1009' name='nUnitClassLibrary.dll' fullname='C:\Users\navb\source\repos\nUnitClassLibrary\nUnitClassLibrary\bin\Debug\nUnitClassLibrary.dll' runstate='Runnable' testcasecount='5'>
+ <properties>
+ <property name='_PID' value='6164' />
+ <property name='_APPDOMAIN' value='domain-71b2ab93-nUnitClassLibrary.dll' />
+ </properties>
+ <test-suite type='TestSuite' id='0-1010' name='nUnitClassLibrary' fullname='nUnitClassLibrary' runstate='Runnable' testcasecount='5'>
+ <test-suite type='TestFixture' id='0-1000' name='Class1' fullname='nUnitClassLibrary.Class1' classname='nUnitClassLibrary.Class1' runstate='Runnable' testcasecount='1'>
+ <properties>
+ <property name='Category' value='BaseClass' />
+ </properties>
+ <test-case id='0-1001' name='nUnitTest' fullname='nUnitClassLibrary.Class1.nUnitTest' methodname='nUnitTest' classname='nUnitClassLibrary.Class1' runstate='Runnable' seed='113395783'>
+ <properties>
+ <property name='Category' value='Base' />
+ </properties>
+ </test-case>
+ </test-suite>
+ <test-suite type='TestFixture' id='0-1002' name='ClassD' fullname='nUnitClassLibrary.ClassD' classname='nUnitClassLibrary.ClassD' runstate='Runnable' testcasecount='2'>
+ <properties>
+ <property name='Category' value='DerivedClass' />
+ <property name='Category' value='BaseClass' />
+ </properties>
+ <test-case id='0-1003' name='dNunitTest' fullname='nUnitClassLibrary.ClassD.dNunitTest' methodname='dNunitTest' classname='nUnitClassLibrary.ClassD' runstate='Runnable' seed='405714082'>
+ <properties>
+ <property name='Category' value='Derived' />
+ </properties>
+ </test-case>
+ <test-case id='0-1004' name='nUnitTest' fullname='nUnitClassLibrary.ClassD.nUnitTest' methodname='nUnitTest' classname='nUnitClassLibrary.Class1' runstate='Runnable' seed='1553985978'>
+ <properties>
+ <property name='Category' value='Base' />
+ </properties>
+ </test-case>
+ </test-suite>
+ <test-suite type='TestFixture' id='0-1005' name='NestedClasses' fullname='nUnitClassLibrary.NestedClasses' classname='nUnitClassLibrary.NestedClasses' runstate='Runnable' testcasecount='1'>
+ <properties>
+ <property name='Category' value='NS1' />
+ </properties>
+ <test-case id='0-1006' name='NC11' fullname='nUnitClassLibrary.NestedClasses.NC11' methodname='NC11' classname='nUnitClassLibrary.NestedClasses' runstate='Runnable' seed='1107340752'>
+ <properties>
+ <property name='Category' value='NS11' />
+ </properties>
+ </test-case>
+ </test-suite>
+ <test-suite type='TestFixture' id='0-1007' name='NestedClasses+NestedClass2' fullname='nUnitClassLibrary.NestedClasses+NestedClass2' classname='nUnitClassLibrary.NestedClasses+NestedClass2' runstate='Runnable' testcasecount='1'>
+ <properties>
+ <property name='Category' value='NS2' />
+ </properties>
+ <test-case id='0-1008' name='NC21' fullname='nUnitClassLibrary.NestedClasses+NestedClass2.NC21' methodname='NC21' classname='nUnitClassLibrary.NestedClasses+NestedClass2' runstate='Runnable' seed='1823789309'>
+ <properties>
+ <property name='Category' value='NS21' />
+ </properties>
+ </test-case>
+ </test-suite>
+ </test-suite>
+ </test-suite>
+</test-run>";
+ #endregion
+
public const string ResultXml =
@"<test-suite
id='121'
@@ -105,6 +163,11 @@ namespace NUnit.VisualStudio.TestAdapter.Tests.Fakes
return XmlHelper.CreateXmlNode(TestXml).SelectSingleNode("test-case");
}
+ public static XmlNodeList GetTestNodes()
+ {
+ return XmlHelper.CreateXmlNode(TestXml2).SelectNodes("//test-case");
+ }
+
public static XmlNode GetResultNode()
{
return XmlHelper.CreateXmlNode(ResultXml).SelectSingleNode("test-case");

diff:
// ***********************************************************************
using System;
+using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
@@ -70,6 +71,37 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.That(parentTrait[0].Value, Is.EqualTo("super"));
}
+ [Test]
+ public void CanMakeTestCaseShouldBuildTraitsCache()
+ {
+ //System.Diagnostics.Debugger.Launch();
+ var xmlNodeList = FakeTestData.GetTestNodes();
+
+ foreach(XmlNode node in xmlNodeList)
+ {
+ var testCase = testConverter.ConvertTestCase(node);
+ }
+
+ var attributesCache = testConverter.AttributesCache;
+
+ // There are 12 ids in the TestXml2, but will be storing only ancestor properties.
+ // Not the leaf node test-case properties.
+ Assert.That(attributesCache.Keys.Count, Is.EqualTo(7));
+
+ // Even though ancestor doesn't have any properties. Will be storing their ids.
+ // So that we will not make call SelectNodes call again.
+ CheckNodesWithNoProperties(attributesCache);
+
+ // Will not be storing leaf nodes test-case nodes in the cache.
+ CheckNoTestCaseNodesExist(attributesCache);
+
+ CheckNodeProperties(attributesCache, "0-1000", new KeyValuePair<string,string>[] { new KeyValuePair<string, string>("Category", "BaseClass") });
+ CheckNodeProperties(attributesCache, "0-1002", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "DerivedClass"), new KeyValuePair<string, string>("Category", "BaseClass") });
+ CheckNodeProperties(attributesCache, "0-1005", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "NS1") });
+ CheckNodeProperties(attributesCache, "0-1007", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "NS2") });
+
+ }
+
[Test]
public void ConvertedTestCaseIsCached()
{
@@ -121,5 +153,33 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
var traitList = testCase.GetTraits().Select(trait => trait.Name + ":" + trait.Value).ToList();
Assert.That(traitList, Is.EquivalentTo(new[] { "Category:super", "Category:cat1", "Priority:medium" }));
}
+
+ private void CheckNodesWithNoProperties(IDictionary<string, List<Trait>> attributesCache)
+ {
+ Assert.That(attributesCache["2"].Count, Is.EqualTo(0));
+ Assert.That(attributesCache["0-1009"].Count, Is.EqualTo(0));
+ Assert.That(attributesCache["0-1010"].Count, Is.EqualTo(0));
+ }
+
+ private void CheckNoTestCaseNodesExist(IDictionary<string, List<Trait>> attributesCache)
+ {
+ Assert.That(!attributesCache.ContainsKey("0-1008"));
+ Assert.That(!attributesCache.ContainsKey("0-1006"));
+ Assert.That(!attributesCache.ContainsKey("0-1004"));
+ Assert.That(!attributesCache.ContainsKey("0-1003"));
+ Assert.That(!attributesCache.ContainsKey("0-1001"));
+ }
+
+ private void CheckNodeProperties(IDictionary<string, List<Trait>> attributesCache, string id, KeyValuePair<string,string>[] kps)
+ {
+ Assert.That(attributesCache.ContainsKey(id));
+ Assert.That(attributesCache[id].Count, Is.EqualTo(kps.Count()));
+ var traits = attributesCache[id];
+
+ foreach(var kp in kps)
+ {
+ Assert.That(traits.Any(t => t.Name == kp.Key && t.Value == kp.Value));
+ }
+ }
}
}


message:
Reading DisableAppDomain Setting from the testplatform runsettings
TestPlatform can send DisableAppDomain setting to adapters.
When DisableAppDomain is true Will set "DomainUsage" to "None"

diff:
@@ -57,6 +57,8 @@ namespace NUnit.VisualStudio.TestAdapter
public bool CollectSourceInformation { get; private set; }
+ public bool DisableAppDomain { get; private set; }
+
#endregion
#region Properties - TestRunParameters
@@ -128,6 +130,7 @@ namespace NUnit.VisualStudio.TestAdapter
TargetFrameworkVersion = GetInnerText(runConfiguration, "TargetFrameworkVersion");
TestAdapterPaths = GetInnerText(runConfiguration, "TestAdapterPaths");
CollectSourceInformation = GetInnerTextAsBool(runConfiguration, "CollectSourceInformation", true);
+ DisableAppDomain = GetInnerTextAsBool(runConfiguration, "DisableAppDomain", false);
TestProperties = new Dictionary<string, string>();
foreach (XmlNode node in doc.SelectNodes("RunSettings/TestRunParameters/Parameter"))
@@ -178,6 +181,12 @@ namespace NUnit.VisualStudio.TestAdapter
DomainUsage = "None";
SynchronousEvents = true;
}
+
+ // If DisableAppDomain settings is passed from the testplatform, set the DomainUsage to None.
+ if(DisableAppDomain)
+ {
+ DomainUsage = "None";
+ }
}
public void SaveRandomSeed(string dirname)

diff:
@@ -50,6 +50,7 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.False(_settings.SynchronousEvents);
Assert.Null(_settings.DomainUsage);
Assert.False(_settings.InProcDataCollectorsAvailable);
+ Assert.IsFalse(_settings.DisableAppDomain);
}
[Test]
@@ -94,6 +95,13 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.That(_settings.CollectSourceInformation, Is.EqualTo(false));
}
+ [Test]
+ public void DisableAppDomainSetting()
+ {
+ _settings.Load("<RunSettings><RunConfiguration><DisableAppDomain>true</DisableAppDomain></RunConfiguration></RunSettings>");
+ Assert.That(_settings.DisableAppDomain, Is.EqualTo(true));
+ }
+
[Test]
public void TestRunParameterSettings()
{

original_message:
Reading DisableAppDomain Setting from the testplatform runsettings

- TestPlatform can send DisableAppDomain setting to adapters.
- When DisableAppDomain is true Will set "DomainUsage" to "None"


message:
Reading other settings that meant for adapters
DisableParallelization
DesignMode : Can be used to figure out if IsRunningUnderIDE.

diff:
@@ -55,10 +55,26 @@ namespace NUnit.VisualStudio.TestAdapter
public string TestAdapterPaths { get; private set; }
+ /// <summary>
+ /// If false, an adapter need not parse symbols to provide test case file, line number
+ /// </summary>
public bool CollectSourceInformation { get; private set; }
+ /// <summary>
+ /// If true, an adapter shouldn't create appdomains to run tests
+ /// </summary>
public bool DisableAppDomain { get; private set; }
+ /// <summary>
+ /// If true, an adapter should disable any test case parallelization
+ /// </summary>
+ public bool DisableParallelization { get; private set; }
+
+ /// <summary>
+ /// True if test run is triggered in an IDE/Editor context.
+ /// </summary>
+ public bool DesignMode { get; private set; }
+
#endregion
#region Properties - TestRunParameters
@@ -131,6 +147,8 @@ namespace NUnit.VisualStudio.TestAdapter
TestAdapterPaths = GetInnerText(runConfiguration, "TestAdapterPaths");
CollectSourceInformation = GetInnerTextAsBool(runConfiguration, "CollectSourceInformation", true);
DisableAppDomain = GetInnerTextAsBool(runConfiguration, "DisableAppDomain", false);
+ DisableParallelization = GetInnerTextAsBool(runConfiguration, "DisableParallelization", false);
+ DesignMode = GetInnerTextAsBool(runConfiguration, "DesignMode", false);
TestProperties = new Dictionary<string, string>();
foreach (XmlNode node in doc.SelectNodes("RunSettings/TestRunParameters/Parameter"))
@@ -187,6 +205,12 @@ namespace NUnit.VisualStudio.TestAdapter
{
DomainUsage = "None";
}
+
+ // If DisableParallelization setting is passed from the testplatform, set the NumberOfTestWorkers to 0
+ if(DisableParallelization)
+ {
+ NumberOfTestWorkers = 0;
+ }
}
public void SaveRandomSeed(string dirname)

diff:
@@ -102,6 +102,20 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.That(_settings.DisableAppDomain, Is.EqualTo(true));
}
+ [Test]
+ public void DisableParallelizationSetting()
+ {
+ _settings.Load("<RunSettings><RunConfiguration><DisableParallelization>true</DisableParallelization></RunConfiguration></RunSettings>");
+ Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(0));
+ }
+
+ [Test]
+ public void DesignModeSetting()
+ {
+ _settings.Load("<RunSettings><RunConfiguration><DesignMode>true</DesignMode></RunConfiguration></RunSettings>");
+ Assert.That(_settings.DesignMode, Is.EqualTo(true));
+ }
+
[Test]
public void TestRunParameterSettings()
{

original_message:
Reading other settings that meant for adapters

- DisableParallelization
- DesignMode : Can be used to figure out if IsRunningUnderIDE.


message: Handled conflicting values for DisableParallelization & NumberOfTestWorkers

diff:
@@ -206,11 +206,8 @@ namespace NUnit.VisualStudio.TestAdapter
DomainUsage = "None";
}
- // If DisableParallelization setting is passed from the testplatform, set the NumberOfTestWorkers to 0
- if(DisableParallelization)
- {
- NumberOfTestWorkers = 0;
- }
+ // Update NumberOfTestWorkers based on the DisableParallelization and NumberOfTestWorkers from runsettings.
+ UpdateNumberOfTestWorkers();
}
public void SaveRandomSeed(string dirname)
@@ -246,6 +243,19 @@ namespace NUnit.VisualStudio.TestAdapter
#region Helper Methods
+ private void UpdateNumberOfTestWorkers()
+ {
+ if(DisableParallelization && NumberOfTestWorkers <=0)
+ {
+ NumberOfTestWorkers = 0;
+ }
+ else if(DisableParallelization && NumberOfTestWorkers > 0)
+ {
+ _logger.Warning(string.Format("DisableParallelization:{0} & NumberOfTestWorkers:{1} are conflicting settings, hence not running in parallel", DisableParallelization, NumberOfTestWorkers));
+ NumberOfTestWorkers = 0;
+ }
+ }
+
private string GetInnerText(XmlNode startNode, string xpath, params string[] validValues)
{
if (startNode != null)

diff:
@@ -5,6 +5,7 @@ using System.Text;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;
using NUnit.Framework;
+using NUnit.VisualStudio.TestAdapter.Tests.Fakes;
namespace NUnit.VisualStudio.TestAdapter.Tests
{
@@ -15,7 +16,7 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
[SetUp]
public void SetUp()
{
- _settings = new AdapterSettings(null);
+ _settings = new AdapterSettings(new TestLogger(new MessageLoggerStub(), 0));
}
[Test]
@@ -111,6 +112,15 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(0));
}
+ [Test]
+ public void UpdateNumberOfTestWorkersWhenConflictingSettings()
+ {
+ _settings.Load("<RunSettings><RunConfiguration><DisableParallelization>true</DisableParallelization></RunConfiguration><NUnit><NumberOfTestWorkers>12</NumberOfTestWorkers></NUnit></RunSettings>");
+
+ // When there's a conflicting values in DisableParallelization and NumberOfTestWorkers. Falling back to DisableParallelization setting.
+ Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(0));
+ }
+
[Test]
public void DesignModeSetting()
{


message: test & fix for assembly level attributes

diff:
@@ -97,20 +97,23 @@ namespace NUnit.VisualStudio.TestAdapter
private static void AddAttributesToCache(IDictionary<string, List<Trait>> propertiesCache, string key, string propertyName, string propertyValue)
{
- if (propertiesCache.ContainsKey(key) && !IsInternalProperty(propertyName, propertyValue))
+ if (propertiesCache.ContainsKey(key))
{
+ if(!IsInternalProperty(propertyName, propertyValue))
propertiesCache[key].Add(new Trait(propertyName, propertyValue));
+ return;
}
- else
- {
+
+
var traits = new List<Trait>();
+
+ // Will add empty list of traits, if the property is internal type. So that we will not make SelectNodes call again.
if (!IsInternalProperty(propertyName, propertyValue))
{
traits.Add(new Trait(propertyName, propertyValue));
}
propertiesCache[key] = traits;
}
- }
public static IEnumerable<NTrait> GetTraits(this TestCase testCase)
{

diff:
@@ -60,6 +60,7 @@ namespace NUnit.VisualStudio.TestAdapter.Tests.Fakes
public const string TestXml2 = @"<test-run id='2' name='nUnitClassLibrary.dll' fullname='C:\Users\navb\source\repos\nUnitClassLibrary\nUnitClassLibrary\bin\Debug\nUnitClassLibrary.dll' testcasecount='5'>
<test-suite type='Assembly' id='0-1009' name='nUnitClassLibrary.dll' fullname='C:\Users\navb\source\repos\nUnitClassLibrary\nUnitClassLibrary\bin\Debug\nUnitClassLibrary.dll' runstate='Runnable' testcasecount='5'>
<properties>
+ <property name='Category' value='AsmCat' />
<property name='_PID' value='6164' />
<property name='_APPDOMAIN' value='domain-71b2ab93-nUnitClassLibrary.dll' />
</properties>

diff:
@@ -74,7 +74,6 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
[Test]
public void CanMakeTestCaseShouldBuildTraitsCache()
{
- //System.Diagnostics.Debugger.Launch();
var xmlNodeList = FakeTestData.GetTestNodes();
foreach(XmlNode node in xmlNodeList)
@@ -95,8 +94,14 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
// Will not be storing leaf nodes test-case nodes in the cache.
CheckNoTestCaseNodesExist(attributesCache);
+ // Checking assembly level attribute.
+ CheckNodeProperties(attributesCache, "0-1009", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "AsmCat") });
+
+ // Checking Class level attributes base class & dervied class
CheckNodeProperties(attributesCache, "0-1000", new KeyValuePair<string,string>[] { new KeyValuePair<string, string>("Category", "BaseClass") });
CheckNodeProperties(attributesCache, "0-1002", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "DerivedClass"), new KeyValuePair<string, string>("Category", "BaseClass") });
+
+ // Checking Nested class attributes.
CheckNodeProperties(attributesCache, "0-1005", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "NS1") });
CheckNodeProperties(attributesCache, "0-1007", new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("Category", "NS2") });
@@ -157,7 +162,6 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
private void CheckNodesWithNoProperties(IDictionary<string, List<Trait>> attributesCache)
{
Assert.That(attributesCache["2"].Count, Is.EqualTo(0));
- Assert.That(attributesCache["0-1009"].Count, Is.EqualTo(0));
Assert.That(attributesCache["0-1010"].Count, Is.EqualTo(0));
}


message: Identifying the DisableParallelization specified or not.

diff:
@@ -90,21 +90,7 @@ namespace NUnit.VisualStudio.TestAdapter
public static bool IsRunningUnderIDE
{
- get
- {
- if (exeName == null)
- {
- Assembly entryAssembly = Assembly.GetEntryAssembly();
- if (entryAssembly != null)
- exeName = entryAssembly.Location;
-
- }
-
- return exeName != null && (
- exeName.Contains("vstest.executionengine") ||
- exeName.Contains("vstest.discoveryengine") ||
- exeName.Contains("TE.ProcessHost"));
- }
+ get; private set;
}
#endregion
@@ -125,6 +111,7 @@ namespace NUnit.VisualStudio.TestAdapter
{
Settings.Load(context);
TestLog.Verbosity = Settings.Verbosity;
+ IsRunningUnderIDE = Settings.DesignMode;
}
catch (Exception e)
{

diff:
@@ -52,7 +52,7 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.Null(_settings.DomainUsage);
Assert.False(_settings.InProcDataCollectorsAvailable);
Assert.IsFalse(_settings.DisableAppDomain);
- Assert.IsFalse(_settings.DisableParallelization);
+ Assert.IsFalse(_settings.DisableParallelization.HasValue);
Assert.IsFalse(_settings.DesignMode);
}
@@ -119,6 +119,12 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
// When there's a conflicting values in DisableParallelization and NumberOfTestWorkers. Falling back to DisableParallelization setting.
Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(0));
+
+ _settings.Load("<RunSettings><RunConfiguration><DisableParallelization>false</DisableParallelization></RunConfiguration><NUnit><NumberOfTestWorkers>0</NumberOfTestWorkers></NUnit></RunSettings>");
+ Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(-1));
+
+ _settings.Load("<RunSettings><RunConfiguration></RunConfiguration><NUnit><NumberOfTestWorkers>12</NumberOfTestWorkers></NUnit></RunSettings>");
+ Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(12));
}
[Test]


message: Updating missing files.

diff:
@@ -68,7 +68,7 @@ namespace NUnit.VisualStudio.TestAdapter
/// <summary>
/// If true, an adapter should disable any test case parallelization
/// </summary>
- public bool DisableParallelization { get; private set; }
+ public bool? DisableParallelization { get; private set; }
/// <summary>
/// True if test run is triggered in an IDE/Editor context.
@@ -147,7 +147,7 @@ namespace NUnit.VisualStudio.TestAdapter
TestAdapterPaths = GetInnerText(runConfiguration, "TestAdapterPaths");
CollectSourceInformation = GetInnerTextAsBool(runConfiguration, "CollectSourceInformation", true);
DisableAppDomain = GetInnerTextAsBool(runConfiguration, "DisableAppDomain", false);
- DisableParallelization = GetInnerTextAsBool(runConfiguration, "DisableParallelization", false);
+ DisableParallelization = GetInnerTextAsNullableBool(runConfiguration, "DisableParallelization");
DesignMode = GetInnerTextAsBool(runConfiguration, "DesignMode", false);
TestProperties = new Dictionary<string, string>();
@@ -245,15 +245,26 @@ namespace NUnit.VisualStudio.TestAdapter
private void UpdateNumberOfTestWorkers()
{
- if(DisableParallelization && NumberOfTestWorkers <=0)
+ if(!DisableParallelization.HasValue)
+ {
+ return;
+ }
+
+ if(DisableParallelization.Value && NumberOfTestWorkers < 0)
{
NumberOfTestWorkers = 0;
}
- else if(DisableParallelization && NumberOfTestWorkers > 0)
+ else if(DisableParallelization.Value && NumberOfTestWorkers > 0)
{
_logger.Warning(string.Format("DisableParallelization:{0} & NumberOfTestWorkers:{1} are conflicting settings, hence not running in parallel", DisableParallelization, NumberOfTestWorkers));
NumberOfTestWorkers = 0;
}
+ else if(!DisableParallelization.Value && NumberOfTestWorkers == 0)
+ {
+ _logger.Warning(string.Format("DisableParallelization:{0} & NumberOfTestWorkers:{1} are conflicting settings, hence running in parallel", DisableParallelization, NumberOfTestWorkers));
+ // Setting it to less than zero, so that framework will decide it.
+ NumberOfTestWorkers = -1;
+ }
}
private string GetInnerText(XmlNode startNode, string xpath, params string[] validValues)
@@ -312,6 +323,16 @@ namespace NUnit.VisualStudio.TestAdapter
return bool.Parse(temp);
}
+ private bool? GetInnerTextAsNullableBool(XmlNode startNode, string xpath)
+ {
+ string temp = GetInnerText(startNode, xpath);
+
+ if (string.IsNullOrEmpty(temp))
+ return null;
+
+ return bool.Parse(temp);
+ }
+
#endregion
}
}

diff:
@@ -90,7 +90,21 @@ namespace NUnit.VisualStudio.TestAdapter
public static bool IsRunningUnderIDE
{
- get; private set;
+ get
+ {
+ if (exeName == null)
+ {
+ Assembly entryAssembly = Assembly.GetEntryAssembly();
+ if (entryAssembly != null)
+ exeName = entryAssembly.Location;
+
+ }
+
+ return exeName != null && (
+ exeName.Contains("vstest.executionengine") ||
+ exeName.Contains("vstest.discoveryengine") ||
+ exeName.Contains("TE.ProcessHost"));
+ }
}
#endregion
@@ -111,7 +125,6 @@ namespace NUnit.VisualStudio.TestAdapter
{
Settings.Load(context);
TestLog.Verbosity = Settings.Verbosity;
- IsRunningUnderIDE = Settings.DesignMode;
}
catch (Exception e)
{

original_message: Updating missing files.


message: Honouring DisableParallelization only if it's True.

diff:
@@ -68,7 +68,7 @@ namespace NUnit.VisualStudio.TestAdapter
/// <summary>
/// If true, an adapter should disable any test case parallelization
/// </summary>
- public bool? DisableParallelization { get; private set; }
+ public bool DisableParallelization { get; private set; }
/// <summary>
/// True if test run is triggered in an IDE/Editor context.
@@ -147,7 +147,7 @@ namespace NUnit.VisualStudio.TestAdapter
TestAdapterPaths = GetInnerText(runConfiguration, "TestAdapterPaths");
CollectSourceInformation = GetInnerTextAsBool(runConfiguration, "CollectSourceInformation", true);
DisableAppDomain = GetInnerTextAsBool(runConfiguration, "DisableAppDomain", false);
- DisableParallelization = GetInnerTextAsNullableBool(runConfiguration, "DisableParallelization");
+ DisableParallelization = GetInnerTextAsBool(runConfiguration, "DisableParallelization", false);
DesignMode = GetInnerTextAsBool(runConfiguration, "DesignMode", false);
TestProperties = new Dictionary<string, string>();
@@ -245,26 +245,16 @@ namespace NUnit.VisualStudio.TestAdapter
private void UpdateNumberOfTestWorkers()
{
- if(!DisableParallelization.HasValue)
- {
- return;
- }
-
- if(DisableParallelization.Value && NumberOfTestWorkers < 0)
+ // Overriding the NumberOfTestWorkers if DisableParallelization is true.
+ if(DisableParallelization && NumberOfTestWorkers < 0)
{
NumberOfTestWorkers = 0;
}
- else if(DisableParallelization.Value && NumberOfTestWorkers > 0)
+ else if(DisableParallelization && NumberOfTestWorkers > 0)
{
_logger.Warning(string.Format("DisableParallelization:{0} & NumberOfTestWorkers:{1} are conflicting settings, hence not running in parallel", DisableParallelization, NumberOfTestWorkers));
NumberOfTestWorkers = 0;
}
- else if(!DisableParallelization.Value && NumberOfTestWorkers == 0)
- {
- _logger.Warning(string.Format("DisableParallelization:{0} & NumberOfTestWorkers:{1} are conflicting settings, hence running in parallel", DisableParallelization, NumberOfTestWorkers));
- // Setting it to less than zero, so that framework will decide it.
- NumberOfTestWorkers = -1;
- }
}
private string GetInnerText(XmlNode startNode, string xpath, params string[] validValues)

diff:
@@ -52,7 +52,7 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
Assert.Null(_settings.DomainUsage);
Assert.False(_settings.InProcDataCollectorsAvailable);
Assert.IsFalse(_settings.DisableAppDomain);
- Assert.IsFalse(_settings.DisableParallelization.HasValue);
+ Assert.IsFalse(_settings.DisableParallelization);
Assert.IsFalse(_settings.DesignMode);
}
@@ -117,12 +117,14 @@ namespace NUnit.VisualStudio.TestAdapter.Tests
{
_settings.Load("<RunSettings><RunConfiguration><DisableParallelization>true</DisableParallelization></RunConfiguration><NUnit><NumberOfTestWorkers>12</NumberOfTestWorkers></NUnit></RunSettings>");
- // When there's a conflicting values in DisableParallelization and NumberOfTestWorkers. Falling back to DisableParallelization setting.
+ // When there's a conflicting values in DisableParallelization and NumberOfTestWorkers. Override the NumberOfTestWorkers.
Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(0));
+ // Do not override the NumberOfTestWorkers when DisableParallelization is False
_settings.Load("<RunSettings><RunConfiguration><DisableParallelization>false</DisableParallelization></RunConfiguration><NUnit><NumberOfTestWorkers>0</NumberOfTestWorkers></NUnit></RunSettings>");
- Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(-1));
+ Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(0));
+ // Do not override the NumberOfTestWorkers when DisableParallelization is not defined
_settings.Load("<RunSettings><RunConfiguration></RunConfiguration><NUnit><NumberOfTestWorkers>12</NumberOfTestWorkers></NUnit></RunSettings>");
Assert.That(_settings.NumberOfTestWorkers, Is.EqualTo(12));
}