ARIA-99 Straightforward end-to-end tests for parser and built-in workflow #67

tliron · 2017-02-03T19:34:14Z

No description provided.

ran-z · 2017-02-07T10:16:34Z

tests/resources/service_templates/node-cellar/types/openstack.yaml

+
+node_types:
+
+  openstack.Instance:


This is a duplication of what we (will need to) have for the Openstack plugin (have yet to translate its plugin.yaml)

All these files are just for testing the TOSCA grammar. That's why they are in /tests/. There is no intention for them to be anything other than that.

right, if these only test the parser then it makes sense,
what I had in mind was that if we're talking true "complete" tests then in the future we'll want to run tests which use the actual plugin as well etc.

ran-z · 2017-02-07T10:17:24Z

tests/resources/service_templates/node-cellar/types/aria.yaml

@@ -0,0 +1,93 @@
+
+policy_types:


Why is this here? This should not be a part of the tests

To test policy type parsing.

lol I understand, but the problem is this yaml import should be part of the extension code and not part of the tests. I realize it's not there right now but I think you might as well have thrown it over there instead right now, no?
Unless I'm missing something, and you think this duplication will remain even once we do have it as part of the extension..?

ran-z · 2017-02-07T10:18:21Z

tests/resources/service_templates/node-cellar/node-cellar.yaml

+  node_cellar:
+    description: >-
+      The repository for the Node Cellar application and its dependencies.
+    url: https://github.com/cloudify-cosmo/nodecellar/archive


is it a good idea to have a direct reference to Cloudify here?

It could just be a dummy URL.

ran-z · 2017-02-07T10:19:19Z

tests/resources/service_templates/node-cellar/node-cellar.yaml

+  template_name: node-cellar
+  template_author: ARIA
+  template_version: '1.0.0'
+  aria_version: '1.0'


we technically never actually called ARIA's first version to be "1.0". actually, on JIRA the version we're supposedly working towards is "0.1.0". not that it really matters here, but thought I'd mention it

Again, this utterly does not matter. It's here to test metadata parsing, to show that we can parse built in fields as well as custom fields.

ran-z · 2017-02-07T11:48:33Z

tests/orchestrator/test_complete.py

+def _workflow(workflow_name):
+    context, _ = consume_node_cellar()
+
+    if workflow_name in BUILTIN_WORKFLOWS:


this section shouldn't be part of the test
the logic needs to sit elsewhere but the CLI in which case the test can test it as well

Since we're rehauling the CLI I am worried about code disappearing. For now I say let's keep this in the test. When we have the new CLI this particular section of code can be redone. I will add a TODO comment here.

ran-z · 2017-02-07T11:49:16Z

tests/orchestrator/test_complete.py

@@ -0,0 +1,58 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more


IMO e2e tests need to be separated from unit/functional tests, and sit under, say, "tests/e2e" or something similar.

I disagree, but it's not too important. Do you want me to move them? I'd rather not use "e2e" because most people will not know what that means. How about "complete"?

I don't mind the name much, didn't really mean for it to be "e2e". I think "system_tests" is a better name than "complete", but i'll leave it up to you - I mostly want them separated from the rest - that, I do think is important.

ran-z · 2017-02-07T11:50:48Z

tests/orchestrator/test_complete.py

+
+
+def test_custom():
+    _workflow('maintenance_on')


I'm not sure we should be testing custom workflows at this time, simply because we haven't quite implemented this feature "for real" yet. It's fine to keep it, I just don't see much benefit for it at the moment.

Since we insist that all our code have tests, I added this.

ran-z · 2017-02-07T11:52:14Z

tests/parser/service_templates.py

+from .utils import (get_uri, create_context, create_consumer)
+
+
+def consume_node_cellar(consumer_class_name='instance', cache=True):


sorry for nitpicking but if this module is designated to be a place for several templates to be used by various tests, I think simply "node_cellar" is a better name in this case :)

I like function names to have verbs in them, because it makes reading code that calls them easier.

ran-z · 2017-02-07T11:53:39Z

tests/parser/test_complete.py

@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more


I think the module name should be changed, possibly to "test_consumers" or so?

ran-z · 2017-02-07T14:59:41Z

tests/complete/test_parser.py

@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.


Actually, I'm not sure these tests should have moved out from where they were at.
"Complete" means end to end tests; While these are not unit tests per se, they are parser specific.
We already have many tests which aren't standard unit tests yet are treated as such. See execution plugin tests for example - those actually run the workflow engine.
On the other hand, the parser might indeed be used regardless of other components of ARIA by other projects, and so it might be ok to leave this here.
Again, I'll leave it for your consideration.

asfbot · 2017-02-08T17:24:54Z

Can one of the admins verify this patch?

…flow

asfgit force-pushed the ARIA-99-e2e-tests branch from aa5df64 to f94bff0 Compare February 3, 2017 20:52

tliron closed this Feb 6, 2017

tliron reopened this Feb 6, 2017

ran-z suggested changes Feb 7, 2017

View reviewed changes

asfgit force-pushed the ARIA-99-e2e-tests branch from 88d6aa2 to e3af53b Compare February 7, 2017 14:41

ran-z reviewed Feb 7, 2017

View reviewed changes

asfgit force-pushed the ARIA-99-e2e-tests branch 2 times, most recently from 9061273 to 344e3ca Compare February 8, 2017 03:17

ARIA-99 Straightforward end-to-end tests for parser and built-in work…

d35d09a

…flow

asfgit force-pushed the ARIA-99-e2e-tests branch from e6695d5 to d35d09a Compare February 8, 2017 17:35

asfgit merged commit d35d09a into master Feb 8, 2017

asfgit deleted the ARIA-99-e2e-tests branch February 9, 2017 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARIA-99 Straightforward end-to-end tests for parser and built-in workflow #67

ARIA-99 Straightforward end-to-end tests for parser and built-in workflow #67

tliron commented Feb 3, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017 •

edited

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

tliron Feb 7, 2017

ran-z Feb 7, 2017

ran-z Feb 7, 2017

asfbot commented Feb 8, 2017

		@@ -0,0 +1,58 @@
		# Licensed to the Apache Software Foundation (ASF) under one or more

		from .utils import (get_uri, create_context, create_consumer)


		def consume_node_cellar(consumer_class_name='instance', cache=True):

		@@ -0,0 +1,40 @@
		# Licensed to the Apache Software Foundation (ASF) under one or more


		node_types:

		openstack.Instance:

ARIA-99 Straightforward end-to-end tests for parser and built-in workflow #67

ARIA-99 Straightforward end-to-end tests for parser and built-in workflow #67

Conversation

tliron commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tliron Feb 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfbot commented Feb 8, 2017

tliron Feb 7, 2017 •

edited