Investigate potential benefits of refactoring factories to only set required data by default to speed up test suite

tbrlpld commented 11 months ago

The test suite currently takes several minutes to complete although we only have ~500 tests.

While I have not exactly traced the timing, I believe that a much of the time maybe coming from the factories. The factories seem to fill all possible fields on a generated object. While this is works for the fake data setup, I think this make the tests slow as many of the models have several layers of related models. This means a lot of data needs to be generated and retrieved during a test, making the test slow.

It would probably be good to spent a little time to investigate if this is actually the case. If my hunch turns out to be accurate, we might want to consider steps to refactor our factories to be minimal and only set required fields by default. Other fields can still be set with factories, but we might benefit from an "opt-in" model, enabled by using Traits.

┆Issue is synchronized with this Jira Task

tbrlpld commented 11 months ago

These are the 50 slowest tests (output generated with pytest --durations 50).

2.57s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::TestBuyersGuidePage::test_serve_page_many_products_logged_in
2.39s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::TestBuyersGuidePage::test_serve_page_many_products
2.12s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::TestBuyersGuidePage::test_get_context_many_products_many_categories
1.51s call     network-api/networkapi/wagtailpages/tests/blog/test_blog_index.py::TestBlogIndex::test_page_size_24_accounts_for_topics_box
1.50s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_categories.py::TestIsBeingUsedProperty::test_usage_property_with_prefetching
1.37s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::TestBuyersGuidePage::test_get_context_many_products_logged_in_user
1.23s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::TestBuyersGuidePage::test_get_context_many_products
1.11s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::WagtailBuyersGuideVoteTest::test_successful_vote_on_translated_page
0.92s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_utils.py::AnnotateProductCategoriesLocalNamesTests::test_can_annotate_local_names
0.89s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_localized_related_products_non_default_locale
0.82s call     network-api/networkapi/wagtailcustomization/tests/test_page_move_redirects.py::PageRedirectTest::test_page_move_creates_redirect_de
0.82s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_utils.py::LocalizeCategoryParentTests::test_can_localize_categories
0.82s call     network-api/networkapi/wagtailpages/tests/blog/test_blog_index.py::TestBlogIndex::test_page_size_12_accounts_for_topics_box
0.81s call     network-api/networkapi/wagtailcustomization/tests/test_page_move_redirects.py::PageRedirectTest::test_page_loads
0.79s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_categories.py::TestIsBeingUsedProperty::test_usage_property_with_translated_pages
0.78s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_product_page_evaluation.py::TestProductPageEvaluation::test_votes_with_translated_page
0.77s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_primary_related_articles_non_default_locale
0.76s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_get_related_articles_non_default_locale
0.76s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_secondary_related_articles_non_default_locale
0.75s call     network-api/networkapi/wagtailcustomization/tests/test_page_move_redirects.py::PageRedirectTest::test_page_move_creates_redirect
0.72s call     network-api/networkapi/wagtailcustomization/tests/test_page_move_redirects.py::PageRedirectTest::test_page_move_creates_redirect_fr
0.71s call     network-api/networkapi/wagtailpages/tests/blog/test_blog_index.py::TestBlogIndexAuthors::test_get_authors_frequent_topics
0.68s call     network-api/networkapi/wagtailpages/tests/libraries/rcc/test_library_page.py::TestRCCLibraryPageFilters::test_filter_multiple_author_profiles
0.68s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_product_page_evaluation.py::CreateEvaluationPostSaveSignalTests::test_that_created_product_page_in_translated_language_syncs_evaluation
0.67s call     network-api/networkapi/wagtailpages/tests/libraries/rcc/test_library_page.py::TestRCCLibraryPageSearch::test_pagination
0.67s call     network-api/networkapi/wagtailpages/tests/libraries/rcc/test_library_page.py::TestRCCLibraryPageFilters::test_pagination
0.66s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_product_with_multiple_categories_shows_cta
0.66s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_product_page_evaluation.py::CreateEvaluationPostSaveSignalTests::test_that_copied_page_gets_evaluation
0.66s call     network-api/networkapi/wagtailpages/tests/blog/test_blog_index.py::TestBlogIndexSearch::test_search_entries_route_loads_first_page_entries_with_query
0.65s call     network-api/networkapi/wagtailpages/tests/blog/test_blog_index.py::TestBlogIndexSearch::test_search_entries_route_loads_second_page_entries_with_query
0.65s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::BuyersGuideViewTest::test_localised_homepage
0.65s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_product_with_multiple_categories_hides_cta
0.65s call     network-api/networkapi/wagtailpages/tests/libraries/rcc/test_library_page.py::TestRCCLibraryPage::test_pagination
0.64s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_product_with_single_enabled_category_shows_cta
0.64s call     network-api/networkapi/wagtailpages/tests/blog/test_blog_index.py::TestBlogIndex::test_page_size_8_unaffected_by_topics_box
0.63s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_product_with_single_disabled_category_hides_cta
0.62s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_localized_related_products
0.60s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_homepage.py::BuyersGuideViewTest::test_category_view_404
0.60s call     network-api/networkapi/wagtailpages/tests/libraries/research_hub/test_library_page.py::TestResearchLibraryPageFilters::test_pagination
0.59s call     network-api/networkapi/wagtailpages/tests/libraries/research_hub/test_library_page.py::TestResearchLibraryPageSearch::test_pagination
0.59s call     network-api/networkapi/wagtailpages/tests/libraries/research_hub/test_library_page.py::TestResearchLibraryPage::test_pagination
0.58s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_editorial_content_index.py::BuyersGuideEditorialContentIndexPageTest::test_hx_request_shows_children_titles
0.58s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_categories.py::TestIsBeingUsedProperty::test_localized_categories_sync_slugs
0.57s call     network-api/networkapi/reports/tests/test_page_type_report.py::PageTypesReportFiltersTests::test_filter_by_locale
0.54s call     network-api/networkapi/donate/tests/test_help_page.py::DonateHelpPageTest::test_page_success
0.52s call     network-api/networkapi/wagtailpages/tests/libraries/rcc/test_detail_page.py::TestRCCLibraryDetailPage::test_localized_authors_returns_localized_profiles_rendered
0.51s call     network-api/networkapi/wagtailpages/tests/buyersguide/test_products.py::TestProductPage::test_primary_related_articles
0.51s call     network-api/networkapi/wagtailpages/tests/publications/test_article.py::ArticePageTests::test_page_loads
0.50s call     network-api/networkapi/wagtailpages/tests/libraries/research_hub/test_library_page.py::TestResearchLibraryPageFilters::test_filter_multiple_author_profiles

Results (193.34s (0:03:13)):
     584 passed
       1 xfailed

So it looks like the setUp and setUpTestData methods are fine. The above report is listing if "setup", "teardown" or "call" of a test is slow. It looks like all the slowness is in the calls.

This does not yet rule out the factories. Many of our tests use the test client to make requests to the pages. If such a page load needs to retrieve a lot of data from the database the test call will be slow. How much data needs to be retrieved might still be related to all the data that the factories are generating by default, even it is not needed in the test.

Of course, we might also run factories in a test call (instead of in a setup method) which will make the test call slow.

jhonatan-lopes commented 11 months ago

Maybe testing it out with an SQLite backend would let us know if the issue is with transferring data from the DB. If the testing time improves after switching the backend, it might be because of that issue.

This might be beyond the scope of this ticket, but it might also be worth also exploring how we can have more sensible testing settings.

tbrlpld commented 11 months ago

@jhonatan-lopes

Maybe testing it out with an SQLite backend would let us know if the issue is with transferring data from the DB. If the testing time improves after switching the backend, it might be because of that issue.

That could be an approach. Apart from that we could pick couple of the slowest tests and run profilers on them (cProfile or pyspy or similar).

This might be beyond the scope of this ticket, but it might also be worth also exploring how we can have more sensible testing settings.

Interesting. Yea, maybe good as a separate ticket. Maybe as part of a new testing speed epic? Do you have examples of settings that are weird?

jhonatan-lopes commented 11 months ago

It's just that the entire settings file right now is used to run the tests. What we usually do is skim that do the bare essentials in a "settings/test.py" inheriting from base settings or something. Some things that might be influencing test speed:

All the logging seems to be enabled during testing
I'm not sure if pattern_library and django-debug-toolbar are disabled on testing if their env vars are true (they might be enabled)
Password validators can be simplified when testing

tbrlpld commented 11 months ago

Yea, we should definitely be testing with settings as close to production as possible (rather than with dev settings).

MozillaFoundation / foundation.mozilla.org

Investigate potential benefits of refactoring factories to only set required data by default to speed up test suite #11543