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

Refactor Upgrader to ensure the new code is reloaded after overwriting old files #3021

Closed
robocoder opened this issue Mar 7, 2012 · 16 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@robocoder
Copy link
Contributor

I hate #1331 workarounds. It's uglifying the #1713 improvements.

We need to make a cleaner break. We need to force a new browser request so that we aren't loading new classes while instances of old classes are in memory.

@robocoder
Copy link
Contributor Author

(In [5988]) refs #1331, refs #3021 - remove #1331 hacks

@robocoder
Copy link
Contributor Author

(In [5991]) fixes #3021, refs #1331 - adds Piwik_View_OneClickDone

During a Piwik software update, there will be instances of old classes
loaded in memory. This is problematic as we will start to instantiate
new classes which may not be backward compatible. This class provides
a clean bridge/transition by forcing a new request.

This class needs to be self-contained, with no external dependencies.

@mattab
Copy link
Member

mattab commented Mar 7, 2012

Every time I updated wordpress I noticed they were doing this and thought "we should do it too" :)

Great idea, well done!! This will save a lot of pain in the future.

@mattab
Copy link
Member

mattab commented Mar 8, 2012

(in the right ticket this time)
Review/questions:

  • Did you test auto-updating from 1.7.1 to trunk?
  • Did you test auto-updating from trunk to an imaginative next release?
  • Did you check that errors/warning are reported when there are some?

Thanks!

@robocoder
Copy link
Contributor Author

I tested 1.0 to trunk.

I instrumented the errors for testing. (Limited time)

We'll be able to test the other scenarios when I get Selenium running on dev6. (I have it working locally.) I've also done some work with Behat/Mink but will defer that until the switch to phpunit.

@mattab
Copy link
Member

mattab commented Mar 8, 2012

Selenium running on dev6. (I have it working locally.)
Excellent news!!

I've also done some work with Behat/Mink but will defer that until the switch to phpunit.
Sounds fun too!

I have looked quickly and it looks interesting. Do you plan to integrate this to improve our current test coverage, or at first to update existing webtests and make it easier to write new web tests?

Tests
Testing the case "Update from trunk to fake next release" is important too (as I assume it triggers different code path)

@mattab
Copy link
Member

mattab commented Mar 8, 2012

Reopening until confirmation that trunk -> fake 1.8 update works

@robocoder
Copy link
Contributor Author

(In [6008]) refs #3021 - form tweaks

@robocoder
Copy link
Contributor Author

(In [6009]) refs #3021 - fix button

@robocoder
Copy link
Contributor Author

Behat/Mink testing won't improve test coverage. Its main benefit is enabling a more natural language for writing acceptance tests.

We've unfortunately lost a lot of web test coverage due to WebTest + jQuery/jQuery-UI incompatibilities. Replacing WebTest with Selenium will be my priority after #3030.

@mattab
Copy link
Member

mattab commented Mar 21, 2012

Todo before closing this ticket:

  • test "Update from trunk to fake next release" is important too (as I assume it triggers different code path)

@vipsoft Is there any other pending task? thx

@mattab
Copy link
Member

mattab commented Mar 28, 2012

For testing auto update easily, see #3071

@mattab
Copy link
Member

mattab commented May 30, 2012

I tested an automatic upgrade from 1.7.1 to 1.8-rc2 but after 1min20, after it unpacked, etc. it redirected to login screen with errors "Login / password not provided"... there was no summary of the auto update. Maybe auto upgrade is partially broken (the UI feedback part) but files seemt o have downloaded and overwrite OK.

@mattab
Copy link
Member

mattab commented May 30, 2012

(In [6404]) Fixes #3071 New config setting to Allow automatic upgrades to Beta or RC releases
[Debug]
allow_upgrades_to_beta = 0

practical to test for Auto upgrades or for users & devs Refs #3021

@mattab
Copy link
Member

mattab commented May 31, 2012

Great work on this one Anthon, clean & few code for an elegant solution to a recurrent issue. Now we don't need the ugly is_class() calls? :)

@mattab
Copy link
Member

mattab commented May 31, 2012

I did a test upgrading from rc4 to rc5 and it worked fine. not sure what hapened in my test before.

It will feel safer with the webtest from 1.0 upgrade ;)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

No branches or pull requests

2 participants