@diosmosis opened this Issue on August 28th 2015 Member

As title.

@diosmosis commented on August 30th 2015 Member

Created a PR here: https://github.com/piwik/travis-scripts/pull/5

@tsteur Can you let me know if you think this would have helped the travis build failure you experienced?

EDIT: once the travis-scripts PR is merged, this issue can be closed.

@tsteur commented on August 31st 2015 Owner

It would have helped for sure. Just wondering if we can let the build succeed in this case or mark as ignore? The tests might otherwise fail for many weeks till the next stable is released. This is not only a bit annoying but will also take a lot of time as various people will over time see the failing build and they all will look into it because it's red. Mainly thinking of you and Matt, they will identify that error quickly re min version is not available yet. Once we release that stable version, a big downside is that we probably won't notice the difference between "the build is failing because min version is not released yet" and "there is actually a problem when running tests against latest stable" since there won't be a change. In this case it goes from "failed" to "failed". Whereas if we let it succeed it would go from "succeeded" to "failed" which is more alarming. If one sees for weeks that this build is red one will start to get used to it over time and maybe ignore failures.

Alternative idea:

  • If latest stable is not released yet, we run against latest beta instead of latest stable
  • If there is no latest beta released yet that matches the minimum version, we let it fail (same logic as in PR)

In this case it is correct to fail the tests as it is implemented now since not such a version exists at all. This is clearly an error. One should require a minimum version where there is at least a beta available.

@diosmosis commented on August 31st 2015 Member

The tests might otherwise fail for many weeks till the next stable is released.

I'm not sure what you mean by this, only one job in a plugin build will force using a different version of Piwik, and that job uses the minimum required Piwik in plugin.json.

If you raise the minimum required Piwik and want to test against that, you have to modify the environment variable in .travis.yml (if you're raising it past the test against version), see eg: https://github.com/piwik/plugin-CustomAlerts/blob/master/.travis.yml#L18

If there's no git tag for that version, then you're telling travis to test against a version that doesn't exist (the code will test against master in that case).

I think in your case, you raised the minimum required Piwik, but didn't know you had to change the .travis.yml file too, which caused it to check out 2.14 which the plugin wasn't compatible w/. Is that accurate?

@tsteur commented on September 1st 2015 Owner

Now I get it, I had no idea one has to take care of PIWIK_LATEST_STABLE_TEST_TARGET manually. I thought it would be done automatically by travis to check for latest stable version. And my suggestion was if PIWIK_LATEST_STABLE_TEST_TARGET does not satisfy the condition by the plugin it should check if the latest beta does, automatically.

Can we somehow make these kinda things automatically by default and only if someone wants to have it hardcoded one can do it too? Maybe in a different issue? It would be very helpful if it worked automatically for developers and if one doesn't have to update .travis.yml etc.

@diosmosis commented on September 1st 2015 Member

Now I get it, I had no idea one has to take care of PIWIK_LATEST_STABLE_TEST_TARGET manually. I thought it would be done automatically by travis to check for latest stable version.

It is meant to be explicit, the developer should explicitly decide what the 'test against' version is. For example, one of the pro plugins is for a client that runs 2.9 as the latest version. We can't test against latest stable since the plugin can't be made compatible for 2.14 and 2.9 simultaneously. And having a build that silently changed the test against version would make it useless when checking if it will work for the client.

As for the other plugins, they are all developed against latest stable. They would never need to change the test against version until its time to release a new stable version. Which is what all plugin developers should be doing. In fact, the only time the minimum required Piwik will be greater than the latest stable is when one of us is developing a plugin against master. If we make changes to core and raise the minimum required Piwik, it could end up being greater than the latest stable, causing the travis error in question. I've worked around this by setting the target to master w/ a TODO to set it to 2.15 once it is released.

Can we somehow make these kinda things automatically by default and only if someone wants to have it hardcoded one can do it too?

The test against version is like the 'maximum supported Piwik' combined w/ the 'specific supported Piwik' (for plugins that are for clients w/ old Piwiks). It has to be explicit, making it automatic would make the build useless since it would automatically start testing against Piwiks that the plugin may not be ready for (ie, latest stable).

We could, as you suggest, only change it when the minimum required > test against, but then the build will silently succeed even though there's a problem w/ the .travis.yml file.

The choice here is not simply do it manually vs. do it automatically, it's do it manually and allow plugin developers to choose which versions to test against and to allow them to make plugin compatible on their own time table, or do it automatically so core developers don't have to be explicit and so plugin builds will immediately and continuously fail after a new Piwik stable is released. Which is doubly problematic for cloud since it makes the build useless between the time when the new stable is released and cloud is updated.

Changes I'll Make

So the above is a bit of an uncompromising stance, perhaps not very helpful. I thought about ways of making it easier to develop against Piwik master w/o making the process different for all pro plugins, can't really think of too much though:

  • I think I can make some small changes to make it more explicit why the error is occurring (eg a better error message).
  • We could also perhaps make a development mode for the plugin build. But I don't know how to detect when a plugin is being developed against master as opposed to latest stable. Could supply an extra parameter to generate:travis-yml but that is also explicit. And once the plugin is released, the .travis.yml file will have to be changed to test against latest stable (since it will be in maintenance mode, not development mode at that point).

    Maybe the test against version could be supplied in plugin.json? Then at least there would be only one file to edit. I wouldn't want to do this until after 2.15 is released, though, since it will require changing every plugin build.

Though honestly, I don't think all this work is necessary just to avoid changing one line in .travis.yml.

@diosmosis commented on September 1st 2015 Member

What I wanted to do when I made the travis-scripts repo was to create a plugin lifecycle management command (ie plugin:lifecycle develop ,plugin:lifecycle release minor/major/patch, plugin:lifecycle new-piwik-stable) etc. Perhaps that would be something to look into soon.

@tsteur commented on September 2nd 2015 Owner

The test against version is like the 'maximum supported Piwik' combined w/ the 'specific supported Piwik' (for plugins that are for clients w/ old Piwiks). It has to be explicit, making it automatic would make the build useless since it would automatically start testing against Piwiks that the plugin may not be ready for (ie, latest stable).

You're right, always testing against latest stable/beta doesn't work. The maximum supported Piwik that you mentioned is I think exactly what I want. I don't get the part why it would be useless? If the plugin is not ready for it yet, I wanna know this and get notified via such a test run. Then, as a plugin developer, I will either make the plugin work with maximum supported Piwik and release a new update or I will change the required Piwik version in plugin.json. Maybe more about this further below.

Maybe the test against version could be supplied in plugin.json? Then at least there would be only one file to edit. I wouldn't want to do this until after 2.15 is released, though, since it will require changing every plugin build.

Yes, the developers do that already when they specify the required/supported Piwik version:

    "require": {
      "piwik": ">=2.15.0-b3"
    },

They specify the minimum required version and the maximum required version (which could be always the latest stable if eg only >2.X is used or a specific min/max version if eg >2.1,<2.3 is used). In this case I do want the version in .travis.yml to change automatically so I get notified when my plugin is no longer compatible with the latest version for some reasons. Eg when CSS changes, Translations, other API's etc. Currently, developers would have to specify the required Piwik version twice. In plugin.json and .travis.yml.

Though honestly, I don't think all this work is necessary just to avoid changing one line in .travis.yml.

I think in reality it will just barely happen that one does it. Developers are maybe not aware of the need of changing it (at least I wasn't aware that one has to change it there so I presume plugin developers do possibly neither) and even if they know it, it's so easy to forget and as long as the build is green everything is good so no need to change anything for them.

All this part about what is the correct required Piwik version is a very critical part see eg the issue I created in https://github.com/piwik/piwik/issues/6149 since it's basically not possible to know for a developer in which versions a plugin will work (maybe will be a bit better once we do https://github.com/piwik/piwik/issues/8547 ) and people easily forget to update the required Piwik version when they work on their plugin. This is why I think it would be very helpful to test against minimum and maximum specified Piwik version automatically. Otherwise users get plugin updates etc that are not compatible with their Piwik anymore and it crashes their Piwik etc. It's not a good experience even if we show safemode screens etc.

I think we need to differentiate a bit between Piwik Pro plugin developers and community developers. While it might work for Pro developers to update it normally it won't really work for community developers I think. Would be really nice to have it automatically their to prevent developers from making mistakes if/when possible.

I'm not saying we need to develop this in this issue, I'd suggest to create a new issue. A first version could be similar to this one and just fail or so if maximum supported Piwik version != the specified in .travis.yml

We could also perhaps make a development mode for the plugin build. But I don't know how to detect when a plugin is being developed against master as opposed to latest stable. Could supply an extra parameter to generate:travis-yml but that is also explicit. And once the plugin is released, the .travis.yml file will have to be changed to test against latest stable (since it will be in maintenance mode, not development mode at that point).

I got the point that always using latest stable etc is not right. Instead we should just look in required Piwik version. A plugin is being developed against master if no maximum version is specified in plugin.json. In this case we should run against latest beta or latest stable though instead of master.

@tsteur commented on September 2nd 2015 Owner

Two more ideas:

  • We should maybe rename PIWIK_LATEST_STABLE_TEST_TARGET. The name is why I thought it would be always the latest stable automatically. In Travis etc one often doesn't see the full variable but only PIWIK_LATEST_STABLE or even if one can see the full variable in travis it still kinda suggests it will be tested against latest stable.
  • An alternative could be also to email plugin developers (at least the once we know or a mailing list etc) to check their plugins re compatibility with latest beta eg 5 days before a new release and we could mention in this email that they have to update .travis.yml to do this.
@diosmosis commented on September 3rd 2015 Member

Merged https://github.com/piwik/travis-scripts/pull/5 (checking that test against version is > than minimum required Piwik for version)

@diosmosis commented on September 3rd 2015 Member

Merged https://github.com/piwik/travis-scripts/pull/7 (renamed PIWIK_LATEST_STABLE_TEST_TARGET to PIWIK_TEST_TARGET + added a comment about the variable in generated .travis.yml files).

@diosmosis commented on September 3rd 2015 Member

This is why I think it would be very helpful to test against minimum

Note: minimum is currently tested against by default

I don't get the part why it would be useless?

I think it would be useless on the whole, because we are the only two people who should develop plugins against master, everyone else (including pro devs) should develop against the lastest stable. And once the LTS cycle is started, they will even if they use Piwik master (since we will develop on branches like 3.X, 4.X, etc.).

A plugin is being developed against master if no maximum version is specified in plugin.json.

The problem I am concerned w/ is, if this is done and we test against this version or master if it is not supplied, then what do we generate when generating a plugin? For easier pro development, the initial plugin.json file should have the maximum set to the latest stable version. For our development (for some plugins) and for whoever else may develop against the unstable master, the maximum should not exist. I don't see a way to make both approaches simple.

An alternative could be also to email plugin developers (at least the once we know or a mailing list etc) to check their plugins re compatibility with latest beta eg 5 days before a new release and we could mention in this email that they have to update .travis.yml to do this.

If this is just for marketplace plugins, we could maybe do this via the piwik-plugins-tests build. Then they would know, even if they don't use travis (or even tests in some cases).

Going to close this issue since I merged some commits in travis-scripts, feel free to continue the discussion.

@tsteur commented on September 3rd 2015 Owner

I think it would be useless on the whole, because we are the only two people who should develop plugins against master, everyone else (including pro devs) should develop against the lastest stable.

I kinda wrote it in the other issue but wanna mention once more that it is important to not set a max supported version for non Piwik Pro developers. Plugin developers should build a plugin and this plugin should not break for any minor or patch release. They should not have to define a maximum version, otherwise we failed being a platform. It's different for major releases where a plugin can break but it should not for any minor or patch releases. This is crucial otherwise we won't have any plugins that support the latest version and if one is using a plugin one couldn't probably upgrade anymore to the latest Piwik version since one is dependent on the Piwik versions that supports that plugin.

This Issue was closed on September 3rd 2015
Powered by GitHub Issue Mirror