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

Method DataIsDifferent() ignores hash keys with undefined values #2080

Closed
bschmalhofer opened this issue Jan 5, 2023 · 5 comments
Closed
Labels
bug Something isn't working as intended unittests Requests wrt unittests
Milestone

Comments

@bschmalhofer
Copy link
Contributor

bschmalhofer commented Jan 5, 2023

This was noticed when migrating a test script from Kernel::System::UnitTest::Driver::IsDeeply() to Test2::V0::is(). The former method does not see a difference when comparing { sample_key => undef } with {}, but sees a difference when comparing {} with
{ sample_key => undef }.

This bug also has repercussions with Kernel::System::UnitTest::Driver::IsDeeply().
Beware that fixing this bug would trigger at least on test failure in scripts/test/DynamicField/ObjectType/Article/ObjectDataGet.t and possibly in other test scripts.

@bschmalhofer bschmalhofer added bug Something isn't working as intended unittests Requests wrt unittests labels Jan 5, 2023
@bschmalhofer bschmalhofer added this to the OTOBO 10.1.7 milestone Jan 5, 2023
@bschmalhofer
Copy link
Contributor Author

See https://github.com/RotherOSS/otobo/tree/issue-%232080-data_is_different for test cases that fail because of this problem.

@bschmalhofer
Copy link
Contributor Author

bschmalhofer commented Jan 5, 2023

The bug concerning DataIsDifferent() surfaced because Kernel::System::Ticket::Article::Backend::Invalid::ArticleGet() returns { SenderType => undef } when there is no article for the queried TicketID/ArticleID. The result can be considered as correct information, but an empty list would be a more sensible result. As far as I see the other backends do return an empty list in that case.

@bschmalhofer
Copy link
Contributor Author

The fallout from fixing DataIsDifferent() is significant but not tremendous.

Test Summary Report
-------------------
/opt/otobo/scripts/test/CommunicationChannel.t                                                            (Wstat: 256 (exited 1) Tests: 67 Failed: 1)
  Failed test:  60
  Non-zero exit status: 1
/opt/otobo/scripts/test/Compile.t                                                                         (Wstat: 0 Tests: 1785 Failed: 0)
  TODO passed:   1147, 1244
/opt/otobo/scripts/test/Daemon/DaemonModules/SchedulerTaskWorker/GenericInterface.t                       (Wstat: 512 (exited 2) Tests: 21 Failed: 2)
  Failed tests:  16, 20
  Non-zero exit status: 2
/opt/otobo/scripts/test/DynamicField/EditFieldValueGet.t                                                  (Wstat: 768 (exited 3) Tests: 144 Failed: 3)
  Failed tests:  38, 41-42
  Non-zero exit status: 3
/opt/otobo/scripts/test/DynamicField/ObjectType/Article/ObjectDataGet.t                                   (Wstat: 512 (exited 2) Tests: 11 Failed: 2)
  Failed tests:  8-9
  Non-zero exit status: 2
/opt/otobo/scripts/test/GenericInterface/Transport/HTTP/SOAP.t                                            (Wstat: 5120 (exited 20) Tests: 58 Failed: 20)
  Failed tests:  13-14, 16, 18, 20, 22, 26, 28-30, 32, 34
                36, 40-43, 46-48
  Non-zero exit status: 20
/opt/otobo/scripts/test/ObjectManager/ObjectParamAdd.t                                                    (Wstat: 512 (exited 2) Tests: 4 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 2
/opt/otobo/scripts/test/Selenium/Agent/Admin/ProcessManagement/AdminProcessManagementTransitionAction.t   (Wstat: 768 (exited 3) Tests: 87 Failed: 3)
  Failed tests:  81, 84, 87
  Non-zero exit status: 3
/opt/otobo/scripts/test/Selenium/TestingMethods.t                                                         (Wstat: 0 Tests: 36 Failed: 0)
  TODO passed:   21, 24, 28
/opt/otobo/scripts/test/Ticket/Article/Backend/Invalid.t                                                  (Wstat: 256 (exited 1) Tests: 24 Failed: 1)
  Failed test:  23
  Non-zero exit status: 1
/opt/otobo/scripts/test/UnitTest.t                                                                        (Wstat: 1024 (exited 4) Tests: 132 Failed: 4)
  Failed tests:  39-40, 63-64
  Non-zero exit status: 4
Files=964, Tests=81050, 5831 wallclock secs (29.03 usr  3.28 sys + 1621.77 cusr 224.86 csys = 1878.94 CPU)
Result: FAIL
ran tests for product OTOBO 10.1.x on host 3f2c2cf4f89a .

bschmalhofer added a commit that referenced this issue Jan 11, 2023
bschmalhofer added a commit that referenced this issue Jan 11, 2023
Add some test cases for the darker corners of DataIsDifferent().
Avoid concationation to an undefined variable.
bschmalhofer added a commit that referenced this issue Jan 11, 2023
on the right side. Even if the value on the left side is an undef.
bschmalhofer added a commit that referenced this issue Jan 11, 2023
now that IsDeeply() is more strict. The extra data
was added by the test script itself.
bschmalhofer added a commit that referenced this issue Jan 11, 2023
is part of the rescheduled task
bschmalhofer added a commit that referenced this issue Jan 11, 2023
inadvertedly the name %Hash2 was used
bschmalhofer added a commit that referenced this issue Jan 11, 2023
No need that the object under test must be called $Kernel::OM.
bschmalhofer added a commit that referenced this issue Jan 11, 2023
The data set up by ObjectParamAdd() did not change,
but the test is now more strict.
bschmalhofer added a commit that referenced this issue Jan 11, 2023
@bschmalhofer
Copy link
Contributor Author

bschmalhofer commented Jan 11, 2023

Fixed the problem of the ignored keys in the compared data structure. Adapted most of the afflicted test scripts. Only scripts/test/GenericInterface/Transport/HTTP/SOAP.t still shows failing tests.

bschmalhofer added a commit that referenced this issue Jan 12, 2023
The actual result data structure did not change,
only the test had become more strict.
bschmalhofer added a commit that referenced this issue Jan 12, 2023
@bschmalhofer
Copy link
Contributor Author

Tests look fine now. The failure in AdminCustomerGroup.t succeeded when run again, looks like a sporadic effect.
Closing this issue.

Test Summary Report
-------------------
/opt/otobo/scripts/test/Compile.t                                                                         (Wstat: 0 Tests: 1785 Failed: 0)
  TODO passed:   1147, 1244
/opt/otobo/scripts/test/Selenium/Agent/Admin/AdminCustomerGroup.t                                         (Wstat: 512 (exited 2) Tests: 38 Failed: 2)
  Failed tests:  37-38
  Non-zero exit status: 2
/opt/otobo/scripts/test/Selenium/Agent/Admin/ProcessManagement/AdminProcessManagementTransitionAction.t   (Wstat: 768 (exited 3) Tests: 87 Failed: 3)
  Failed tests:  81, 84, 87
  Non-zero exit status: 3
/opt/otobo/scripts/test/Selenium/TestingMethods.t                                                         (Wstat: 0 Tests: 36 Failed: 0)
  TODO passed:   21, 24, 28
Files=964, Tests=80997, 5733 wallclock secs (28.46 usr  3.31 sys + 1593.32 cusr 219.57 csys = 1844.66 CPU)
Result: FAIL
ran tests for product OTOBO 10.1.x on host 08502d0fa80f .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended unittests Requests wrt unittests
Projects
None yet
Development

No branches or pull requests

1 participant