Dependency Graph

Dependency Graph
related to related to child of child of duplicate of duplicate of

View Issue Details

IDProjectCategoryView StatusLast Update
16692Bug reportsOtherpublic2020-10-05 08:36
Reportergabrieljenik Assigned Togabrieljenik  
PrioritynoneSeveritypartial_block 
Status closedResolutionfixed 
Product Version3.23.5 
Target Version3.22.13 
Summary16692: Clear process after test script is failing
DescriptionThis is when it happened: https://github.com/LimeSurvey/LimeSurvey/pull/1586

Survey Imports can't be reused among test scripts
It is not that they need to. Still this triggered the error.
`
Last test, changing the survey file, worked good.
Not sure how that made a difference. I will double check.

Also, smell, something is wrong with the cleanup.
I think the problem is we should be using static sometimes, and we are using self.
`
Steps To ReproduceRun testChangeQuestionTemplate and EditQuestionTest in the same run.
TagsNo tags attached.
Complete LimeSurvey version number (& build)3.22.13
I will donate to the project if issue is resolvedNo
Browser
Database & DB-VersionMysql
Server OS (if known)
Webserver software & version (if known)
PHP Versionphp

Relationships

related to 16714 closedgabrieljenik Clear process after test script is failing 

Activities

ollehar

ollehar

2020-09-24 11:39

administrator   ~59943

> Survey Imports can't be reused among test scripts

Not sure they SHOULD be reused, honestly. If one test needs to change the survey, other unrelated tests might brake, which makes a fragile test suite.
gabrieljenik

gabrieljenik

2020-09-24 14:30

manager   ~59944

I agree. I just named like that as a way to test if clearing after test is done right.
It is not a must to be reused. Agree. Even better if not.
But that is a clear sign of clear up being done right.

I will rename it though.
gabrieljenik

gabrieljenik

2020-09-24 14:33

manager   ~59945

Does it worth to rename all current survey files and deduplicate if any is shared among 2 tests?
gabrieljenik

gabrieljenik

2020-10-01 15:04

manager   ~60026

The issue is caused by 'findByPkCache':
1) Test A imports survey 123456. The process involves using findByPK, and that methods saves the survey in a cache.
2) Test A finishes and the survey is deleted.
3) Test B imports survey 123456 again. This is a whole new survey. However, findByPk finds it in the cache, and returns the cached object.
4) Test B now has a copy of survey 123456 that is different from the last imported survey (groups and questions IDs differ).

Solved by clearing the findByPkCache in TestBaseClass::importSurvey() and clearing cache on Survey::delete()

https://github.com/LimeSurvey/LimeSurvey/pull/1612
gabrieljenik

gabrieljenik

2020-10-01 15:10

manager   ~60027

Besides running travis, deleting a survey should be a step on the manual testing
ollehar

ollehar

2020-10-01 15:10

administrator   ~60028

The cache! It's always the cache. :(
gabrieljenik

gabrieljenik

2020-10-02 18:59

manager   ~60056

Fix committed to 3.x-LTS branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=30539
lime_release_bot

lime_release_bot

2020-10-05 08:36

administrator   ~60065

Fixed in Release 3.23.7+201006

Related Changesets

LimeSurvey: 3.x-LTS d99f229f

2020-10-01 15:00:08

gabrieljenik

Details Diff
Fixed issue 16692: Clear process after test script is failing

Cache was not properly cleared in between tests neither after survey deletion
Affected Issues
16692
mod - application/models/Survey.php Diff File
mod - tests/TestBaseClass.php Diff File

LimeSurvey: 3.x-LTS 358bf088

2020-10-02 15:16:56

gabrieljenik

Details Diff
Fixed issue 16692: Clear process after test script is failing

Code fix
Affected Issues
16692
mod - application/models/Survey.php Diff File

LimeSurvey: 3.x-LTS 167b11f4

2020-10-02 18:58:58

ollehar


Committer: GitHub Details Diff
Fixed issue 16692: Clear process after test script is failing Affected Issues
16692
mod - application/models/Survey.php Diff File
mod - tests/TestBaseClass.php Diff File
mod - tests/controllers/ChangeQuestionTemplateTest.php Diff File
rm - tests/data/surveys/limesurvey_survey_573387.lss Diff File

Issue History

Date Modified Username Field Change
2020-09-23 19:46 gabrieljenik New Issue
2020-09-23 19:46 gabrieljenik Status new => assigned
2020-09-23 19:46 gabrieljenik Assigned To => gabrieljenik
2020-09-23 19:48 gabrieljenik Steps to Reproduce Updated View Revisions
2020-09-24 11:39 ollehar Note Added: 59943
2020-09-24 14:30 gabrieljenik Note Added: 59944
2020-09-24 14:31 gabrieljenik Summary Survey Imports can't be reused among test scripts => Clar process after test script is failing
2020-09-24 14:31 gabrieljenik Description Updated View Revisions
2020-09-24 14:33 gabrieljenik Note Added: 59945
2020-10-01 14:51 gabrieljenik Summary Clar process after test script is failing => Clear process after test script is failing
2020-10-01 15:04 gabrieljenik Note Added: 60026
2020-10-01 15:10 gabrieljenik Note Added: 60027
2020-10-01 15:10 ollehar Note Added: 60028
2020-10-01 15:13 gabrieljenik Issue cloned: 16714
2020-10-01 15:13 gabrieljenik Relationship added related to 16714
2020-10-02 18:59 ollehar Changeset attached => LimeSurvey 3.x-LTS 167b11f4
2020-10-02 18:59 gabrieljenik Changeset attached => LimeSurvey 3.x-LTS 358bf088
2020-10-02 18:59 gabrieljenik Changeset attached => LimeSurvey 3.x-LTS d99f229f
2020-10-02 18:59 gabrieljenik Note Added: 60056
2020-10-02 18:59 gabrieljenik Resolution open => fixed
2020-10-05 08:36 lime_release_bot Note Added: 60065
2020-10-05 08:36 lime_release_bot Status assigned => closed