Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test failures due to new ObjectDependencies #1088

Closed
bschmalhofer opened this issue Jun 15, 2021 · 7 comments
Closed

Fix test failures due to new ObjectDependencies #1088

bschmalhofer opened this issue Jun 15, 2021 · 7 comments

Comments

@bschmalhofer
Copy link
Contributor

Looks like some new failures sneaked in:

/opt/otobo/scripts/test/DynamicField/DateTranslation.t (Wstat: 1280 Tests: 23 Failed: 5)
Failed tests: 7, 11, 15, 19, 23
Non-zero exit status: 5
/opt/otobo/scripts/test/DynamicField/ObjectType/Article/ObjectDataGet.t (Wstat: 512 Tests: 11 Failed: 2)
Failed tests: 10-11
Non-zero exit status: 2
/opt/otobo/scripts/test/DynamicField/ObjectType/CustomerCompany/ObjectDataGet.t (Wstat: 512 Tests: 7 Failed: 2)
Failed tests: 6-7
Non-zero exit status: 2
/opt/otobo/scripts/test/DynamicField/ObjectType/CustomerUser/ObjectDataGet.t (Wstat: 512 Tests: 7 Failed: 2)
Failed tests: 6-7
Non-zero exit status: 2
/opt/otobo/scripts/test/DynamicField/ObjectType/Ticket/ObjectDataGet.t (Wstat: 256 Tests: 7 Failed: 1)
Failed test: 7
Non-zero exit status: 1
/opt/otobo/scripts/test/SysConfig/ValueType/Entity/EntityLookupFromWebRequest.t (Wstat: 1024 Tests: 15 Failed: 4)
Failed tests: 12-15
Non-zero exit status: 4
/opt/otobo/scripts/test/Ticket/Event/LockAfterCreate.t (Wstat: 1792 Tests: 19 Failed: 7)
Failed tests: 10, 12, 14, 16-19
Non-zero exit status: 7

Originally posted by @bschmalhofer in #1011 (comment)

@bschmalhofer
Copy link
Contributor Author

The following message in the test output gives a hint of what is going on:

ERROR: OTOBO-otobo.UnitTest-10 Perl: 5.34.0 OS: linux Time: Sat Jun 12 18:03:40 2021

Message: Unable to determine object id for object name and type CustomerCompany!

Traceback (4053):
Module: Kernel::System::DynamicField::ObjectType::CustomerCompany::ObjectDataGet Line: 188
Module: /opt/otobo/scripts/test/DynamicField/ObjectType/CustomerCompany/ObjectDataGet.t Line: 172

DBD::mysql::db do failed: Lock wait timeout exceeded; try restarting transaction at /opt/otobo/Kernel/System/DB.pm line 610.
ERROR: OTOBO-otobo.UnitTest-10 Perl: 5.34.0 OS: linux Time: Sat Jun 12 18:04:31 2021

Message: Lock wait timeout exceeded; try restarting transaction, SQL: '
INSERT INTO dynamic_field_obj_id_name
(object_name, object_type)
VALUES
(?, ?)'

Traceback (4053):
Module: Kernel::System::DynamicField::ObjectMappingCreate Line: 1385
Module: Kernel::System::DynamicField::ObjectType::CustomerCompany::ObjectDataGet Line: 184
Module: /opt/otobo/scripts/test/DynamicField/ObjectType/CustomerCompany/ObjectDataGet.t Line: 172

The message mentions database transactions. This is consistent, as all newly failing test scripts have RestoreDatabase => 1 in the parameter list for K::S::UnitTest::Helper object. RestoreDatabase indicates that the script usually runs in a single transaction. The easiest way to have different transactions in the script is to have at least two different database connections. The DB object is managed by the object manager, so it can be assumed that the object manager creates two different DB objects. A reason for that is, when a DB object is discarded. This checks out, as all newly failing scripts call the Discard method for Kernel::System::Web::Request. Why would discarding the ParamObject discard the DBObject? The DBObject depends on Kernel::System::Log which in turn depends on Kernel::System::Web::Request. Why didn*t the failures before? Because the ParamObject was added to @Kernel::System::Log;;ObjectDependencies only on 2021-06-11 in the commit 9702d06.

The fix should be simple. Remove Kernel::System::Web::Request from @Kernel::System::Log;;ObjectDependencies . The dependency is not deep anyways, as the state of the LogObject does not depend on the ParamObject,

@svenoe
Copy link
Contributor

svenoe commented Jun 16, 2021

I know that you are not happy with the current way object dependencies are handled, but I'm totally against introducing exceptions to the main code, just to make some test scripts run which built on assumptions that are not generally true.

I propose to discuss the object dependencies at some point in a larger round.

(Edit: Also this new dependency actually will influence quite a lot of stuff, as Kernel::System::Log is used almost everywhere...so I there definitely is reason to discuss this aside from those tests...)

@bschmalhofer
Copy link
Contributor Author

How can this be an exception to the main code, when I added the dependency of Kernel::System::Log on Kerrnel::System::Web;;Request just 5 days ago?

bschmalhofer added a commit that referenced this issue Jun 16, 2021
There were also bad effects: discarding Kernel::System::Web::Request led to
new database connection which led at least to test failures.
@bschmalhofer
Copy link
Contributor Author

The fix looks fine, we are back to 0 non-TODO failures.

bschmalhofer added a commit that referenced this issue Jun 18, 2021
This will be used by CodePolicy as a blacklist of modules that are
excempted from the Perl::ObjectDependencies check.
@bschmalhofer
Copy link
Contributor Author

bschmalhofer commented Jun 18, 2021

Discussed this with @svenoe and @kess-net . Fully disabling the CodePolicy for Perl::ObjectDependencies has been ruled out. The solution is to support a blacklist called @SoftObjectDependencies that list the object that are not hard dependencies.
TODO:

  • set up @SoftObjectDependencies where the policy Perl::ObjectDependencies is currently deactivated
  • run the complete test suite
  • adapt the CodePolicy, making excemptions for the blacklist, and check the OTOBO code

@bschmalhofer
Copy link
Contributor Author

TODO for the Codepolicy: Consider the array @SoftObjectDependencies as a blacklist for OTOBO/Perl/ObjectDependencies.pm.

bschmalhofer added a commit that referenced this issue Jun 19, 2021
…_weirdness

Issue #1088: remove unneeded dependencies
@bschmalhofer
Copy link
Contributor Author

Tests and CodePolicy look fine. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants