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

Improve upgrades system, add tests #1048

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improve upgrades system, add tests #1048

wants to merge 3 commits into from

Conversation

remram44
Copy link
Member

@remram44 remram44 commented Apr 7, 2015

This branch has more contrived tests for the upgrades system. Most don't pass.

  • test_upgrade1: This upgrades from version 0.2 to 0.4. There is no explicit upgrade to 0.4 to 0.5, but an automatic upgrade would succeed; however it is not attempted.
  • test_upgrade2: This chains two upgrades, with no ambiguity.
  • test_upgrade3: Here there are multiple possible upgrade from 0.2: either to 0.3 (then 0.5), or directly to 0.5. The upgrade to 0.3 is preferred, this is wrong.
  • test_upgrade4: Here several automatic upgrade would be needed. Module is 0.0, would need to get to 0.1 to upgrade to 0.2, then auto to 0.3 to upgrade to 0.4, then auto to 0.5.

I'm not entirely sure what can be done and what we want to support (specially in terms of automatic upgrades). Discussion is probably needed here.

This doesn't yet cover the problem @rexissimus ran into on VTK functions, #1047. It might be a different issue fixable separately.
#1017 can probably fixed without handling all of this, although the upgrades won't be as robust.

@remram44
Copy link
Member Author

remram44 commented Apr 8, 2015

Also, the remapping functions (passed as function_remap, src_port_remap, etc) don't get the controller, which leads to weird workarounds like the global _controller in vtk's upgrade code. But I don't know if we can change this without breaking compatibility (maybe pass it for new-style upgrades, UpgradeModuleRemap objects, and not for old ones, dict?)

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

Successfully merging this pull request may close these issues.

None yet

1 participant