Project

General

Profile

Actions

action #72082

closed

Reduce test runtime, e.g. less reliance on test fixtures or test database instances

Added by okurz about 4 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
Feature requests
Target version:
Start date:
2020-09-18
Due date:
% Done:

0%

Estimated time:

Description

Motivation

Our tests can run very long which affects feedback cycles. We have many integration tests always initializing a test database with schema. Maybe we can improve by relying less on test fixtures or even completely on test database instances

Acceptance criteria

  • AC1: All tests have been reviewed for their use of fixtures and the test database
  • AC2: Test databases are only instantiated where explicitly needed
  • AC3: Test databases are only initialized with data that is actually needed for specific tests

Suggestions

  • Find all tests that instantiate a test database with fixtures and try instead to use initialization of only necessary data
  • Find all tests that instantiate a test database and try instead to use instantiation of data objects directly instead of using a database
  • Optional: Rework tests to not need a database at all anymore

Related issues 2 (0 open2 closed)

Related to openQA Project (public) - action #90371: Warnings "Subroutine JSON::PP::Boolean::(0+ redefined"Resolvedtinita2021-03-19

Actions
Copied from openQA Project (public) - action #71500: Potential optimization by skipping deployment checks in our tests (all except explicit deployment check tests)Resolvedokurz2020-09-18

Actions
Actions #1

Updated by okurz about 4 years ago

  • Copied from action #71500: Potential optimization by skipping deployment checks in our tests (all except explicit deployment check tests) added
Actions #2

Updated by okurz about 4 years ago

Actions #3

Updated by livdywan about 4 years ago

okurz wrote:

We are down on the use of fixtures in most tests now, and the redundant initialization of the db in Selenium tests is also gone.

That last PR is still an open draft. Are you planning to pursue this further?
Arguably it's a cosmetic fix since it's not removing fixtures but merely adjusting the defaults. So leaving it as-is would make sense to me as well.

Actions #4

Updated by okurz about 4 years ago

cdywan wrote:

That last PR is still an open draft. Are you planning to pursue this further?

yeah, you know how it is with "Low" tickets :) Eventually I would still like to have that in.

Actions #5

Updated by okurz about 4 years ago

https://github.com/os-autoinst/openQA/pull/3421 is merged meanwhile. I did work on https://github.com/os-autoinst/openQA/pull/3416 but then was annoyed by circleCI failing often (handled in other ticket). I looked into this the past days and continued with tiny steps.

Actions #6

Updated by livdywan almost 4 years ago

Discussing this in the weekly we've identified 3 types of issues:

  • Interdependencies of fixtures
  • Fixtures from perl hashes are painfully slow - consider straight SQL (suggested by @kraih)
  • Subroutine JSON::PP::Boolean::(++ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48. unclear what's causing this
Actions #7

Updated by okurz almost 4 years ago

  • Status changed from Feedback to In Progress
Actions #8

Updated by okurz almost 4 years ago

After we discussed the topic today I managed to fix one little issue so added commit https://github.com/os-autoinst/openQA/pull/3416/commits/f2c0ba668d442bf91e2fef4e7eab4f65336e8bf8 onto the PR. Based on test results and the fresh knowledge within the team now I am feeling motivated to bring other issues up to the team more quickly :)

Actions #9

Updated by okurz almost 4 years ago

  • Status changed from In Progress to Feedback
Actions #10

Updated by okurz almost 4 years ago

  • Status changed from Feedback to Resolved

PR merged.

What I did is find all tests that instantiate a test database with fixtures and try instead to use initialization of only necessary data. Partially also instead of a test database we instead use instantiation of data objects directly instead of using a database. Well, what we did not proceed much with though is to rework more tests to not need a database at all anymore but that's ok.

Actions #11

Updated by tinita over 3 years ago

cdywan wrote:

  • Subroutine JSON::PP::Boolean::(++ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48. unclear what's causing this

I debugged a little bit.

Mentioned already in https://progress.opensuse.org/issues/38264, also related https://progress.opensuse.org/issues/29048

https://github.com/rurban/Cpanel-JSON-XS/issues/65 (closed but unfixed)

perl -wE 'use Cpanel::JSON::XS (); use JSON::PP ()'
Subroutine JSON::PP::Boolean::(++ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Subroutine JSON::PP::Boolean::(0+ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Subroutine JSON::PP::Boolean::(-- redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.

Happens only with Devel::Cover and JSON format.

Same with JSON::MaybeXS, which Devel::Cover uses

perl -wE 'use JSON::MaybeXS (); use JSON::PP ()'
Subroutine JSON::PP::Boolean::(0+ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Subroutine JSON::PP::Boolean::(-- redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Subroutine JSON::PP::Boolean::(++ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.

Can be avoided by loading JSON::PP before everything else:

export PERL5OPT="-MJSON::PP -MDevel::Cover...

Unclear why we only see it in the fullstack tests. Maybe JSON::PP is indirectly used in openQA somewhere before Cpanel::JSON::XS, while in os-autoinst code Cpanel::JSON::XS is used first.
But that would also explain why -MJSON::PP "fixes" it, but a use JSON::PP at the top of the t/full-stack.t does not (because the problem lies in os-autoinst code).

Oh, and just for the record, if you're wondering where that warning comes from, a tiny reproducer:

perl -wE'
package Foo;
use overload "+" => sub { say 23 };
use overload "+" => sub { say 23 };
'
Subroutine Foo::(+ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Actions #12

Updated by okurz over 3 years ago

ok, cool. Thanks for the context and the reminder about the other tickets where we have seen this. So in case you think there is anything related to JSON::PP we can do to improve or prevent further future confusing issues then please open a ticket or create a PR or something

Actions #13

Updated by livdywan over 3 years ago

tinita wrote:

Same with JSON::MaybeXS, which Devel::Cover uses

perl -wE 'use JSON::MaybeXS (); use JSON::PP ()'
Subroutine JSON::PP::Boolean::(0+ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Subroutine JSON::PP::Boolean::(-- redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.
Subroutine JSON::PP::Boolean::(++ redefined at /usr/lib/perl5/5.26.1/overload.pm line 48.

Can be avoided by loading JSON::PP before everything else:

export PERL5OPT="-MJSON::PP -MDevel::Cover...

Unclear why we only see it in the fullstack tests. Maybe JSON::PP is indirectly used in openQA somewhere before Cpanel::JSON::XS, while in os-autoinst code Cpanel::JSON::XS is used first.
But that would also explain why -MJSON::PP "fixes" it, but a use JSON::PP at the top of the t/full-stack.t does not (because the problem lies in os-autoinst code).

What I read there is something uses both JSON::MaybeXS and JSON::PP but if JSON::PP comes first, it's fine.

I would suggest a new ticket about this rather than this one which is about test runtime.

Actions #14

Updated by tinita over 3 years ago

  • Related to action #90371: Warnings "Subroutine JSON::PP::Boolean::(0+ redefined" added
Actions #15

Updated by tinita over 3 years ago

I created https://progress.opensuse.org/issues/90371
I only mentioned it here because the warning was mentioned.

Actions

Also available in: Atom PDF