View Issue Details

Related ChangesetsJump to NotesJump to History
This bug affects 1 person(s).
 6
IDProjectCategoryView StatusDate SubmittedLast Update
16692Bug reportsOtherpublic2020-09-23 19:462020-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
Description

This 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 Reproduce

Run testChangeQuestionTemplate and EditQuestionTest in the same run.

TagsNo tags attached.
Bug heat6
Complete LimeSurvey version number (& build)3.22.13
I will donate to the project if issue is resolvedNo
Browser
Database type & versionMysql
Server OS (if known)
Webserver software & version (if known)
PHP Versionphp

Relationships

Relationship GraphDependency Graph
related to 16714 closedgabrieljenik Clear process after test script is failing 

Users monitoring this issue

There are no users monitoring this issue.

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 17:00

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 17:16

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 20: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

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
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
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