Repository: saleor/saleor
Pull request: Delete duplicated product_list fixture
Delete duplicated product_list fixture.
Update product_list with data from duplicated fixture.
Updated tests.
Fixes #1369 

I want to merge this change because...

We have two product_list fixtures in conftest.py

### Pull Request Checklist


1. [x] Privileged views and APIs are guarded by proper permission checks.
1. [x] All visible strings are translated with proper context.
1. [x] All data-formatting is locale-aware (dates, numbers, and so on).
1. [x] Database queries are optimized and the number of queries is constant.
1. [x] The changes are tested.
1. [x] The code is documented (docstrings, project documentation).
1. [x] Python code quality checks pass: `pycodestyle`, `pydocstyle`, `pylint`.
1. [x] JavaScript code quality checks pass: `eslint`.


Problem statement:
Duplicated pytest fixture product_list
We have duplicated 'product_list' fixture in conftest.py.

### What I'm trying to achieve

I want to have only one fixture which returns product list.

### Steps to reproduce the problem

This is not affecting any of the functionalities.

### What I expected to happen

Fixture returning a list of products.

### What happened instead/how it failed

Fixture returned a list of products from other fixture.

(Please include a stack trace if this problem results in a crash.)

Hints from issue discussion:

Commit message: Delete duplicated product_list fixture

Delete duplicated product_list fixture.
Update product_list with data from duplicated fixture.
Updated tests.

Patch to review:
diff --git a/tests/conftest.py b/tests/conftest.py
index 79712001d02..cd65395912d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -283,15 +283,20 @@ def product_list(product_class, default_category):
 
     product_1 = Product.objects.create(
         name='Test product 1', price=Decimal('10.00'),
-        product_class=product_class, attributes=attributes)
+        product_class=product_class, attributes=attributes, is_published=True)
     product_1.categories.add(default_category)
 
     product_2 = Product.objects.create(
         name='Test product 2', price=Decimal('20.00'),
-        product_class=product_class, attributes=attributes)
+        product_class=product_class, attributes=attributes, is_published=False)
     product_2.categories.add(default_category)
 
-    return [product_1, product_2]
+    product_3 = Product.objects.create(
+        name='Test product 3', price=Decimal('20.00'),
+        product_class=product_class, attributes=attributes, is_published=True)
+    product_3.categories.add(default_category)
+
+    return [product_1, product_2, product_3]
 
 
 @pytest.fixture
@@ -481,18 +486,6 @@ def authorization_key(db, site_settings):
         password='Password')
 
 
-@pytest.fixture
-def product_list(product_class, default_category):
-    product_1 = Product.objects.create(
-        name='Test product 1', price=Decimal('10.00'),
-        product_class=product_class, is_published=True)
-    product_1.categories.add(default_category)
-    product_2 = Product.objects.create(
-        name='Test product 2', price=Decimal('20.00'),
-        product_class=product_class, is_published=False)
-    return [product_1, product_2]
-
-
 @pytest.fixture
 def permission_view_staff():
     return Permission.objects.get(codename='view_staff')
diff --git a/tests/dashboard/test_product.py b/tests/dashboard/test_product.py
index 9e68cfb9930..6eae001e2bd 100644
--- a/tests/dashboard/test_product.py
+++ b/tests/dashboard/test_product.py
@@ -542,7 +542,7 @@ def test_product_bulk_update_form_can_unpublish_products(product_list):
 
 def test_product_list_filters(admin_client, product_list):
     db_products = Product.objects.all()
-    assert len(db_products) == 2
+    assert len(db_products) == 3
     data = {'price_1': [''], 'price_0': [''], 'is_featured': [''],
             'name': ['Test'], 'sort_by': [''], 'is_published': ['']}
     url = reverse('dashboard:product-list')
@@ -575,7 +575,8 @@ def test_product_list_filters_is_published(
     url = reverse('dashboard:product-list')
     response = admin_client.get(url, data)
     assert response.status_code == 200
-    assert list(response.context['filter'].qs) == [product_list[0]]
+    result = list(response.context['filter'].qs)
+    assert result == [product_list[0], product_list[2]]
 
 
 def test_product_list_filters_no_results(admin_client, product_list):
diff --git a/tests/test_product.py b/tests/test_product.py
index 402332566c6..228ef47b085 100644
--- a/tests/test_product.py
+++ b/tests/test_product.py
@@ -426,7 +426,7 @@ def test_product_filter_form(authorized_client, product_in_stock,
 def test_product_filter_sorted_by_price_descending(
     authorized_client, product_list, default_category):
     products = (models.Product.objects.all()
-                .filter(categories__name=default_category)
+                .filter(categories__name=default_category, is_published=True)
                 .order_by('-price'))
     url = reverse(
         'product:category', kwargs={'path': default_category.slug,


Code review comments:
diff_hunk: @@ -542,7 +542,7 @@ def test_product_bulk_update_form_can_unpublish_products(product_list):
 
 def test_product_list_filters(admin_client, product_list):
     db_products = Product.objects.all()
-    assert len(db_products) == 2
+    assert len(db_products) == 3
path: tests/dashboard/test_product.py
text: @user1:
I'm not sure if this line is needed. Number of products bases on a fixture structure, which could be treated as a constant.

@author:
hmmm looks like debuging assertion, deleted