action #72082
closedReduce test runtime, e.g. less reliance on test fixtures or test database instances
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
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
Updated by okurz about 4 years ago
- https://github.com/os-autoinst/openQA/pull/3414 for an extracted method (merged)
- https://github.com/os-autoinst/openQA/pull/3421 t: Remove obsolete test job database entry in all initializations
- https://github.com/os-autoinst/openQA/pull/3416 t: Skip fixtures by default to prevent excessive runtime
Updated by livdywan almost 4 years ago
okurz wrote:
- https://github.com/os-autoinst/openQA/pull/3414 for an extracted method (merged)
- https://github.com/os-autoinst/openQA/pull/3421 t: Remove obsolete test job database entry in all initializations
- https://github.com/os-autoinst/openQA/pull/3416 t: Skip fixtures by default to prevent excessive runtime
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.
Updated by okurz almost 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.
Updated by okurz almost 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.
Updated by livdywan over 3 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
Updated by okurz over 3 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 :)
Updated by okurz over 3 years ago
- Status changed from In Progress to Feedback
https://github.com/os-autoinst/openQA/pull/3416 ready for review
Updated by okurz over 3 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.
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.
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
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 ause JSON::PP
at the top of thet/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.
Updated by tinita over 3 years ago
- Related to action #90371: Warnings "Subroutine JSON::PP::Boolean::(0+ redefined" added
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.