[review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[Sujith H]

From: Sujith Haridasan <Sujith_Haridasan@mentor.com>

Inorder to make sure if layer name which we import is already
available in toaster database, a test case has been included.
This would help user to identify if layer name which has been
provided in the import layer web page already exists or not.

Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
---
 bitbake/lib/toaster/toastergui/tests.py | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/bitbake/lib/toaster/toastergui/tests.py b/bitbake/lib/toaster/toastergui/tests.py
index 77e80fe..9e444ae 100644
--- a/bitbake/lib/toaster/toastergui/tests.py
+++ b/bitbake/lib/toaster/toastergui/tests.py
@@ -75,3 +75,34 @@ class XHRDataTypeAheadTestCase(ProvisionedLayersProjectTestCase):
         self.assertTrue(len(data["list"]) > 0)
 
         self.assertTrue(XHRDataTypeAheadTestCase.LAYER_NAME in map(lambda x: x["name"], data["list"]))
+
+
+class XHRImportLayerTestCase(ProvisionedLayersProjectTestCase):
+
+    def setUp(self):
+        super(XHRImportLayerTestCase, self).setUp()
+        self.assertTrue(self.lv in self.project.compatible_layerversions())
+
+    def test_import(self):
+        #Test for importing an already existing layer
+        data = {'vcs_url' : "git://git.example.com/test" ,
+                'name' : "base-layer", 'git_ref': "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
+                'project_id': 1, 'dir_path' : "/path/in/repository"}
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertEqual(result.status_code,200)
+        self.assertTrue("hint-layer-exists-with-different-url" in result.content)
+
+        #Test to verify import of a layer successful
+        data['name'] = "meta-oe"
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertTrue('"error": "ok"' in result.content)
+
+        #Test for html tag in the data
+        data['<'] = "testing html tag"
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertTrue('"error": "Invalid character <"' in result.content)
+
+        #Empty data passed
+        data = {}
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertTrue('"error": "Missing parameters; requires vcs_url, name, git_ref and project_id"' in result.content)
-- 
1.9.1

Re: [review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[sujith h]

[-- Attachment #1: Type: text/plain, Size: 3007 bytes --]

Hi Michael,

I have updated the patch with the feedback shared by you. Kindly let me
know if more changes are required.

Thanks,
Sujith H

On Thu, Aug 6, 2015 at 9:11 PM, Sujith H <sujith.h@gmail.com> wrote:

> From: Sujith Haridasan <Sujith_Haridasan@mentor.com>
>
> Inorder to make sure if layer name which we import is already
> available in toaster database, a test case has been included.
> This would help user to identify if layer name which has been
> provided in the import layer web page already exists or not.
>
> Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
> Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
> ---
>  bitbake/lib/toaster/toastergui/tests.py | 31
> +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/bitbake/lib/toaster/toastergui/tests.py
> b/bitbake/lib/toaster/toastergui/tests.py
> index 77e80fe..9e444ae 100644
> --- a/bitbake/lib/toaster/toastergui/tests.py
> +++ b/bitbake/lib/toaster/toastergui/tests.py
> @@ -75,3 +75,34 @@ class
> XHRDataTypeAheadTestCase(ProvisionedLayersProjectTestCase):
>          self.assertTrue(len(data["list"]) > 0)
>
>          self.assertTrue(XHRDataTypeAheadTestCase.LAYER_NAME in map(lambda
> x: x["name"], data["list"]))
> +
> +
> +class XHRImportLayerTestCase(ProvisionedLayersProjectTestCase):
> +
> +    def setUp(self):
> +        super(XHRImportLayerTestCase, self).setUp()
> +        self.assertTrue(self.lv in
> self.project.compatible_layerversions())
> +
> +    def test_import(self):
> +        #Test for importing an already existing layer
> +        data = {'vcs_url' : "git://git.example.com/test" ,
> +                'name' : "base-layer", 'git_ref':
> "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
> +                'project_id': 1, 'dir_path' : "/path/in/repository"}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertEqual(result.status_code,200)
> +        self.assertTrue("hint-layer-exists-with-different-url" in
> result.content)
> +
> +        #Test to verify import of a layer successful
> +        data['name'] = "meta-oe"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "ok"' in result.content)
> +
> +        #Test for html tag in the data
> +        data['<'] = "testing html tag"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Invalid character <"' in
> result.content)
> +
> +        #Empty data passed
> +        data = {}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Missing parameters; requires vcs_url,
> name, git_ref and project_id"' in result.content)
> --
> 1.9.1
>
>


-- 
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info

[-- Attachment #2: Type: text/html, Size: 4499 bytes --]

Re: [review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[Ed Bartosh]

Hi Sujith,

Can you rebase your patch on top of ed/toaster/fix-toastergui-tests?

And please check that your code doesn't bring new pylint warnings.

I'm running pylint this way:
pylint --load-plugins pylint_django toastergui/tests.py

And it's scores for tests.py code is 10.

On Thu, Aug 06, 2015 at 09:11:28PM +0530, Sujith H wrote:
> From: Sujith Haridasan <Sujith_Haridasan@mentor.com>
> 
> Inorder to make sure if layer name which we import is already
> available in toaster database, a test case has been included.
> This would help user to identify if layer name which has been
> provided in the import layer web page already exists or not.
> 
> Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
> Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
> ---
>  bitbake/lib/toaster/toastergui/tests.py | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/bitbake/lib/toaster/toastergui/tests.py b/bitbake/lib/toaster/toastergui/tests.py
> index 77e80fe..9e444ae 100644
> --- a/bitbake/lib/toaster/toastergui/tests.py
> +++ b/bitbake/lib/toaster/toastergui/tests.py
> @@ -75,3 +75,34 @@ class XHRDataTypeAheadTestCase(ProvisionedLayersProjectTestCase):
>          self.assertTrue(len(data["list"]) > 0)
>  
>          self.assertTrue(XHRDataTypeAheadTestCase.LAYER_NAME in map(lambda x: x["name"], data["list"]))
> +
> +
> +class XHRImportLayerTestCase(ProvisionedLayersProjectTestCase):
> +
> +    def setUp(self):
> +        super(XHRImportLayerTestCase, self).setUp()
> +        self.assertTrue(self.lv in self.project.compatible_layerversions())
> +
> +    def test_import(self):
> +        #Test for importing an already existing layer
> +        data = {'vcs_url' : "git://git.example.com/test" ,
> +                'name' : "base-layer", 'git_ref': "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
> +                'project_id': 1, 'dir_path' : "/path/in/repository"}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertEqual(result.status_code,200)
> +        self.assertTrue("hint-layer-exists-with-different-url" in result.content)
> +
> +        #Test to verify import of a layer successful
> +        data['name'] = "meta-oe"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "ok"' in result.content)
> +
> +        #Test for html tag in the data
> +        data['<'] = "testing html tag"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Invalid character <"' in result.content)
> +
> +        #Empty data passed
> +        data = {}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Missing parameters; requires vcs_url, name, git_ref and project_id"' in result.content)
> -- 
> 1.9.1
> 
> -- 
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster

-- 
--
Regards,
Ed

[review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[Sujith H]

Inorder to make sure if layer name which we import is already
available in toaster database, a test case has been included.
This would help user to identify if layer name which has been
provided in the import layer web page already exists or not.

Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
---
 bitbake/lib/toaster/toastergui/tests.py | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/bitbake/lib/toaster/toastergui/tests.py b/bitbake/lib/toaster/toastergui/tests.py
index 93a85ac..2986e57 100644
--- a/bitbake/lib/toaster/toastergui/tests.py
+++ b/bitbake/lib/toaster/toastergui/tests.py
@@ -148,3 +148,35 @@ class ViewTests(TestCase):
             # After "typeing" the alpabet we should have result true
             # from each of the urls
             self.assertTrue(results)
+
+class XHRImportLayerTestCase(ViewTests):
+    """ Tests to verify xhr_importlayer API"""
+    def setUp(self):
+        super(XHRImportLayerTestCase, self).setUp()
+
+    def test_import(self):
+        """Tests to verify import layer already exist, if the imported layer is ok, response
+        for empty data, response for html tag in data"""
+        #Test for importing an already existing layer
+        data = {'vcs_url' : "git://git.example.com/test",
+                'name' : "base-layer", 'git_ref': "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
+                'project_id': 1, 'dir_path' : "/path/in/repository"}
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertEqual(result.status_code, 200)
+        self.assertTrue("hint-layer-exists-with-different-url" in result.content)
+
+        #Test to verify import of a layer successful
+        data['name'] = "meta-oe"
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertTrue('"error": "ok"' in result.content)
+
+        #Test for html tag in the data
+        data['<'] = "testing html tag"
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertTrue('"error": "Invalid character <"' in result.content)
+
+        #Empty data passed
+        data = {}
+        result = self.client.post(reverse('xhr_importlayer'), data)
+        self.assertTrue('"error": "Missing parameters; requires vcs_url, \
+                        name, git_ref and project_id"' in result.content)
-- 
1.9.1

Re: [review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[sujith h]

[-- Attachment #1: Type: text/plain, Size: 3114 bytes --]

Hi Ed,

I have rebased the patch on branch d/toaster/fix-toastergui-tests.

I ran : pylint --load-plugins pylint_django toastergui/tests.py; and got a
score of 10.

Let me know if more changes are required.

Thanks,
Sujith H

On Mon, Aug 10, 2015 at 2:33 PM, Sujith H <sujith.h@gmail.com> wrote:

> Inorder to make sure if layer name which we import is already
> available in toaster database, a test case has been included.
> This would help user to identify if layer name which has been
> provided in the import layer web page already exists or not.
>
> Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
> Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
> ---
>  bitbake/lib/toaster/toastergui/tests.py | 32
> ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/bitbake/lib/toaster/toastergui/tests.py
> b/bitbake/lib/toaster/toastergui/tests.py
> index 93a85ac..2986e57 100644
> --- a/bitbake/lib/toaster/toastergui/tests.py
> +++ b/bitbake/lib/toaster/toastergui/tests.py
> @@ -148,3 +148,35 @@ class ViewTests(TestCase):
>              # After "typeing" the alpabet we should have result true
>              # from each of the urls
>              self.assertTrue(results)
> +
> +class XHRImportLayerTestCase(ViewTests):
> +    """ Tests to verify xhr_importlayer API"""
> +    def setUp(self):
> +        super(XHRImportLayerTestCase, self).setUp()
> +
> +    def test_import(self):
> +        """Tests to verify import layer already exist, if the imported
> layer is ok, response
> +        for empty data, response for html tag in data"""
> +        #Test for importing an already existing layer
> +        data = {'vcs_url' : "git://git.example.com/test",
> +                'name' : "base-layer", 'git_ref':
> "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
> +                'project_id': 1, 'dir_path' : "/path/in/repository"}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertEqual(result.status_code, 200)
> +        self.assertTrue("hint-layer-exists-with-different-url" in
> result.content)
> +
> +        #Test to verify import of a layer successful
> +        data['name'] = "meta-oe"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "ok"' in result.content)
> +
> +        #Test for html tag in the data
> +        data['<'] = "testing html tag"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Invalid character <"' in
> result.content)
> +
> +        #Empty data passed
> +        data = {}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Missing parameters; requires vcs_url, \
> +                        name, git_ref and project_id"' in result.content)
> --
> 1.9.1
>
>


-- 
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info

[-- Attachment #2: Type: text/html, Size: 4469 bytes --]

Re: [review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[Ed Bartosh]

Hi Sujith,

Thank you for updating your patch.

My comments are below.

On Mon, Aug 10, 2015 at 02:33:10PM +0530, Sujith H wrote:
> Inorder to make sure if layer name which we import is already
> available in toaster database, a test case has been included.
> This would help user to identify if layer name which has been
> provided in the import layer web page already exists or not.
> 
> Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
> Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
> ---
>  bitbake/lib/toaster/toastergui/tests.py | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/bitbake/lib/toaster/toastergui/tests.py b/bitbake/lib/toaster/toastergui/tests.py
> index 93a85ac..2986e57 100644
> --- a/bitbake/lib/toaster/toastergui/tests.py
> +++ b/bitbake/lib/toaster/toastergui/tests.py
> @@ -148,3 +148,35 @@ class ViewTests(TestCase):
>              # After "typeing" the alpabet we should have result true
>              # from each of the urls
>              self.assertTrue(results)
> +
> +class XHRImportLayerTestCase(ViewTests):
> +    """ Tests to verify xhr_importlayer API"""
> +    def setUp(self):
> +        super(XHRImportLayerTestCase, self).setUp()
Do you really need a new class for your test case?
If you add it to ViewTests you don't need above 4 lines.

> +
> +    def test_import(self):
> +        """Tests to verify import layer already exist, if the imported layer is ok, response
> +        for empty data, response for html tag in data"""
Please, split dosctring to subject line and the body to make output of
./manage.py test -v2 toastergui meaningful.

> +        #Test for importing an already existing layer
> +        data = {'vcs_url' : "git://git.example.com/test",
I'd suggest to rename data->args.
> +                'name' : "base-layer", 'git_ref': "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
> +                'project_id': 1, 'dir_path' : "/path/in/repository"}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertEqual(result.status_code, 200)
> +        self.assertTrue("hint-layer-exists-with-different-url" in result.content)
It would be better to convert result from json using json.load as it's
done in other test cases, i.e.
data = json.loads(response.content)
self.assertNotEqual(data["error"], "ok")

> +        #Test to verify import of a layer successful
> +        data['name'] = "meta-oe"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "ok"' in result.content)
> +
> +        #Test for html tag in the data
> +        data['<'] = "testing html tag"
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Invalid character <"' in result.content)
> +
> +        #Empty data passed
> +        data = {}
> +        result = self.client.post(reverse('xhr_importlayer'), data)
> +        self.assertTrue('"error": "Missing parameters; requires vcs_url, \
> +                        name, git_ref and project_id"' in result.content)
Would it be enough to check that error is not 'ok'? If you change a
message you will not have to change test code.

Please, make your code similar to the existing code. Use the same
vairable names, checks, etc. It would make test code look and behave in
a consistent way.

--
Regards,
Ed

Re: [review-request][PATCH][v2] toastergui: Add tests for xhr_importlayer

[sujith h]

[-- Attachment #1: Type: text/plain, Size: 3996 bytes --]

On Mon, Aug 10, 2015 at 9:56 PM, Ed Bartosh <ed.bartosh@linux.intel.com>
wrote:

> Hi Sujith,
>
> Thank you for updating your patch.
>
> My comments are below.
>
> On Mon, Aug 10, 2015 at 02:33:10PM +0530, Sujith H wrote:
> > Inorder to make sure if layer name which we import is already
> > available in toaster database, a test case has been included.
> > This would help user to identify if layer name which has been
> > provided in the import layer web page already exists or not.
> >
> > Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>
> > Signed-off-by: Sujith Haridasan <Sujith_Haridasan@mentor.com>
> > ---
> >  bitbake/lib/toaster/toastergui/tests.py | 32
> ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/bitbake/lib/toaster/toastergui/tests.py
> b/bitbake/lib/toaster/toastergui/tests.py
> > index 93a85ac..2986e57 100644
> > --- a/bitbake/lib/toaster/toastergui/tests.py
> > +++ b/bitbake/lib/toaster/toastergui/tests.py
> > @@ -148,3 +148,35 @@ class ViewTests(TestCase):
> >              # After "typeing" the alpabet we should have result true
> >              # from each of the urls
> >              self.assertTrue(results)
> > +
> > +class XHRImportLayerTestCase(ViewTests):
> > +    """ Tests to verify xhr_importlayer API"""
> > +    def setUp(self):
> > +        super(XHRImportLayerTestCase, self).setUp()
> Do you really need a new class for your test case?
> If you add it to ViewTests you don't need above 4 lines.
>
> > +
> > +    def test_import(self):
> > +        """Tests to verify import layer already exist, if the imported
> layer is ok, response
> > +        for empty data, response for html tag in data"""
> Please, split dosctring to subject line and the body to make output of
> ./manage.py test -v2 toastergui meaningful.
>
> > +        #Test for importing an already existing layer
> > +        data = {'vcs_url' : "git://git.example.com/test",
> I'd suggest to rename data->args.
> > +                'name' : "base-layer", 'git_ref':
> "c12b9596afd236116b25ce26dbe0d793de9dc7ce",
> > +                'project_id': 1, 'dir_path' : "/path/in/repository"}
> > +        result = self.client.post(reverse('xhr_importlayer'), data)
> > +        self.assertEqual(result.status_code, 200)
> > +        self.assertTrue("hint-layer-exists-with-different-url" in
> result.content)
> It would be better to convert result from json using json.load as it's
> done in other test cases, i.e.
> data = json.loads(response.content)
> self.assertNotEqual(data["error"], "ok")
>
> > +        #Test to verify import of a layer successful
> > +        data['name'] = "meta-oe"
> > +        result = self.client.post(reverse('xhr_importlayer'), data)
> > +        self.assertTrue('"error": "ok"' in result.content)
> > +
> > +        #Test for html tag in the data
> > +        data['<'] = "testing html tag"
> > +        result = self.client.post(reverse('xhr_importlayer'), data)
> > +        self.assertTrue('"error": "Invalid character <"' in
> result.content)
> > +
> > +        #Empty data passed
> > +        data = {}
> > +        result = self.client.post(reverse('xhr_importlayer'), data)
> > +        self.assertTrue('"error": "Missing parameters; requires
> vcs_url, \
> > +                        name, git_ref and project_id"' in
> result.content)
> Would it be enough to check that error is not 'ok'? If you change a
> message you will not have to change test code.
>
> Please, make your code similar to the existing code. Use the same
> vairable names, checks, etc. It would make test code look and behave in
> a consistent way.
>

Thanks for detailed feed back. I will make modifications accordingly and
re-submit the patch.

Thanks,
Sujith H

-- 
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info

[-- Attachment #2: Type: text/html, Size: 5610 bytes --]