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

Yii2 model is lost when doing functional test and the action returns redirect response #3961

Closed
biserantonov opened this Issue Jan 26, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@biserantonov

biserantonov commented Jan 26, 2017

Bug is caused by pull request #3834 in v.2.2.8. I'm testing an action which redirects to itself in certain condition. However before the redirect is done the transaction is roll backed and the model that I use for testing is lost.

@Naktibalda Naktibalda added the Yii label Jan 26, 2017

@Naktibalda Naktibalda changed the title from model is lost when doing functional test and the action returns redirect response to Yii2 model is lost when doing functional test and the action returns redirect response Jan 26, 2017

@ddinchev

This comment has been minimized.

Show comment
Hide comment
@ddinchev

ddinchev Jan 27, 2017

Contributor

This caused a lot of frustration. Tests that check that something has happened after a redirect, are broken.

  1. Actor fills form with correct data.
  2. Actor submits form.
  3. Backend saves the record and immediately redirects to the record view page. Record is deleted because of #3834
  4. We check if record is present based on the id recovered from URL, or the submitted test data, and test fails.
Contributor

ddinchev commented Jan 27, 2017

This caused a lot of frustration. Tests that check that something has happened after a redirect, are broken.

  1. Actor fills form with correct data.
  2. Actor submits form.
  3. Backend saves the record and immediately redirects to the record view page. Record is deleted because of #3834
  4. We check if record is present based on the id recovered from URL, or the submitted test data, and test fails.

iRipVanWinkle added a commit to iRipVanWinkle/Codeception that referenced this issue Jan 31, 2017

@StalkAlex

This comment has been minimized.

Show comment
Hide comment
@StalkAlex

StalkAlex Jan 31, 2017

Guys, really? Again? Wasting couple of days to find out why half of functional tests become strangely broken. This isn't fisrt time already though. Bugs during development, ok I understand it, but it's almost impossible to wait another month waiting new release and living with ugly kludges. So fix commit is here, it's working. Please merge it and make another release as soon as possible.

StalkAlex commented Jan 31, 2017

Guys, really? Again? Wasting couple of days to find out why half of functional tests become strangely broken. This isn't fisrt time already though. Bugs during development, ok I understand it, but it's almost impossible to wait another month waiting new release and living with ugly kludges. So fix commit is here, it's working. Please merge it and make another release as soon as possible.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jan 31, 2017

Collaborator

Which fix do you mean?

Collaborator

samdark commented Jan 31, 2017

Which fix do you mean?

@StalkAlex

This comment has been minimized.

Show comment
Hide comment
@StalkAlex

StalkAlex commented Jan 31, 2017

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jan 31, 2017

Collaborator

That is basically a rollback of another fix for a valid issue.

Collaborator

samdark commented Jan 31, 2017

That is basically a rollback of another fix for a valid issue.

@StalkAlex

This comment has been minimized.

Show comment
Hide comment
@StalkAlex

StalkAlex Jan 31, 2017

#3834 ? I think this was related too #3769 . At least previous variant worked properly in "all-green" scenario.

StalkAlex commented Jan 31, 2017

#3834 ? I think this was related too #3769 . At least previous variant worked properly in "all-green" scenario.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jan 31, 2017

Collaborator

In your case yes but since that issue was reported, it was preventing someone else tests from working.

Collaborator

samdark commented Jan 31, 2017

In your case yes but since that issue was reported, it was preventing someone else tests from working.

@StalkAlex

This comment has been minimized.

Show comment
Hide comment
@StalkAlex

StalkAlex Jan 31, 2017

@samdark ok, sorry. Just as I said not the first time.

StalkAlex commented Jan 31, 2017

@samdark ok, sorry. Just as I said not the first time.

@iRipVanWinkle

This comment has been minimized.

Show comment
Hide comment
@iRipVanWinkle

iRipVanWinkle Jan 31, 2017

Contributor

@samdark I can't reproduce this bug #3834. Because my transaction closes of config property cleanup, I think the property enough.

And if this bug exists we can move method of closing transaction here: #L153

            } elseif (!$e instanceof ExitException) {
                // for exceptions not related to Http, we pass them to Codeception
                $this->rollBackTransaction(); // added
                $this->resetApplication();
                throw $e;
            }
Contributor

iRipVanWinkle commented Jan 31, 2017

@samdark I can't reproduce this bug #3834. Because my transaction closes of config property cleanup, I think the property enough.

And if this bug exists we can move method of closing transaction here: #L153

            } elseif (!$e instanceof ExitException) {
                // for exceptions not related to Http, we pass them to Codeception
                $this->rollBackTransaction(); // added
                $this->resetApplication();
                throw $e;
            }

@DavertMik DavertMik closed this in 4f9d4c4 Feb 3, 2017

chris1312 added a commit to chris1312/Codeception that referenced this issue Jun 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment