Defect #3059
closed
Fix tests on current develop
100%
Description
On current develop branch doesn't pass all tests
Local run by Eclipse IDE (maybe different order of tests):
Run by Jenkins:
Files
Related issues
Updated by Ondřej Kopr almost 3 years ago
- Related to Defect #3013: Fix tests on LTS version added
Updated by Tomáš Doischer almost 3 years ago
- Status changed from New to In Progress
- Assignee set to Tomáš Doischer
Updated by Tomáš Doischer almost 3 years ago
In the develop branch, I've not been able to replicate the failing test either in Eclipse or in the command line (Tests run: 151, Failures: 0, Errors: 0, Skipped: 3). I've tried running both `mvn clean install -Dspring.profiles.active=test` and `mvn clean install -Dmaven.wagon.http.ssl.insecure=true -Ptest -Dspring.profiles.active=test verify` (this is close to what Jenkins is using, only without publishing and uploading to sonar).
Some tests are likely dependent on the local environment. I will take a look at those mentioned here and try to find an issue. Likely, some tests are not cleaning their resources. Since by default JUnit tests run in unpredictable order, this can cause problems. BTW (good to know), this can be changed (https://www.baeldung.com/junit-5-test-order) but it is likely not a good solution in most cases.
Updated by Tomáš Doischer almost 3 years ago
I used the option to specify the test order to simulate the behavior above in the Eclipse screenshot. Tests are now failing.
Updated by Tomáš Doischer almost 3 years ago
- % Done changed from 0 to 60
I managed to get the tests running in CreateAutoRolesBasedOnUsersRolesTest.
The null pointer errors were happening when a system (database) was created and the password for the system was saved. The entity had no id of the attribute (password) that was saved. This was a completely ridiculous error.
I tried variously cleaning all sorts of stuff, deleting what was created in a test after the test was finished. This is hard (a lot of stuff is created implicitly) but basically did not help. I tried adding the @DirtiesContext annotation (https://www.baeldung.com/spring-dirtiescontext) which didn't do anything other than cause the tests to run incredibly slowly.
Finally, I tried removing the @Transactional annotation from the test cases which somewhat helped. All tests passed except one which had wrong values.
But this led me to this article: https://www.marcobehler.com/2014/06/25/should-my-tests-be-transactional. It argues that the @Transactional annotation is not always the right approach and that it has its risks. I tried a suggested alternative and made the entire class @Transactional and at end of each test, I call `entityManager.flush();`. With this, everything passes, runs as supposed to, and nothing needs to be deleted.
A WIP commit: https://github.com/bcvsolutions/czechidm-extras/commit/509ff9efe602583ae0a58ce41f736cd70081b64b
I'm not entirely sure why this solution works better, and I want to look at other tests in extras.
Updated by Tomáš Doischer almost 3 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Tomáš Doischer to Peter Štrunc
- % Done changed from 60 to 100
I went over other tests in extras and if they used the @Transactional annotation over many methods, I replaced it. One of those tests was the ContractStartNotificationTaskTest which was reported as failing in this ticket. This should make the test more robust and it doesn't negatively affect test performance. All tests now pass.
Apart from this, in this PR, I added a new method from the product acc TestHelper. I didn't end up using it but it seems potentially quite useful and since the extras TestHelper is mainly a copy of the AccTestHelper, it seemed appropriate to add it.
This PR is cleaned, i. e., there is no fixed order of tests but if you want to simulate the order shown in this ticket, you can easily add it (see previous commits).
PR: https://github.com/bcvsolutions/czechidm-extras/pull/75
@sourek, can you please give me feedback?
Updated by Roman Kučera over 2 years ago
- Status changed from Needs feedback to Resolved
- Assignee changed from Peter Štrunc to Tomáš Doischer
- Target version set to 3.5.0
LGTM, thx.
Updated by Roman Kučera over 2 years ago
We tested it, and under current develop (with merged branch) it's still failing
[ERROR] ContractChangeNotificationTaskTest.testTreeNodeFilterEav:184->testSuccessfullyProcessed:528 expected:<1> but was:<0>
[ERROR] ContractStartNotificationTaskTest.testProjectionFilter:178 expected:<1> but was:<9>
[ERROR] ContractStartNotificationTaskTest.testSystemFilter:212 expected:<1> but was:<9>
[ERROR] CreateAutoRolesBasedOnUsersRolesTest.testAutoRoleCreationWithoutSystemMultipleUsers:266 expected:<2> but was:<9>
[ERROR] CreateAutoRolesBasedOnUsersRolesTest.testAutoRoleCreationWithoutSystemOneUser:210 expected:<2> but was:<9>
[ERROR] LastContractEndNotificationTaskTest.testProjectionFilter:434 expected:<1> but was:<3>
[ERROR] LastContractEndNotificationTaskTest.testSystemFilter:467 expected:<1> but was:<3>
I'll look into it
Updated by Peter Štrunc over 2 years ago
- Status changed from Resolved to Closed