Issue 4540 - Add Platform Specific Branch Support to createNightlies.py

sitescripts/extensions/test/test_createNightlies.py

Index: sitescripts/extensions/test/test_createNightlies.py
===================================================================
new file mode 100644
--- /dev/null
+++ b/sitescripts/extensions/test/test_createNightlies.py
@@ -0,0 +1,37 @@
+# This file is part of the Adblock Plus web scripts,
+# Copyright (C) 2006-2016 Eyeo GmbH
+#
+# Adblock Plus is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# Adblock Plus is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests for create nightlies script."""
+import pytest
+
+from sitescripts.extensions.bin import createNightlies
+from sitescripts.utils import get_config
+
+
+@pytest.fixture()
+def config(hg_dir):
+    config = get_config()
+    config.type = 'safari'
+    config.repository = str(hg_dir.join('adblockplusnightly'))
+    return config
+
+
+@pytest.fixture()
+def nightlybuild(config):
+    return createNightlies.NightlyBuild(config)
+
+
+def test_nightly_object_bookmark(nightlybuild):

[Vasily Kuznetsov, 2016/10/25 16:04:33]

It seems that this test only checks how the nightly build is configured and only
in the case when the `abpxxx_bookmark` option is present. I think this is not
sufficient to cover the changes you're making, so it would be good to also check
that:

1. If the bookmark is absent, the build is configured to use `master`. 
2. The build actually works and uses the correct revision in both of those
cases.

Part 1 should be pretty easy to set. Part 2 could get pretty hairy if you try to
mock everything that is needed for the build to work. I think one way to avoid
"mocking the world" is to just check that `build.hasChanges()` is correct, that
`build.getChanges()` returns the right changes and that `build.copyRepository()`
produces the copy of the right revision.

When you work on part 2 you will probably end up with more tests because you
need to check situations when the bookmark is not on the last commit, etc. This
might slow down the tests since `hg_dir` takes a while to be created. Then it
will make sense to convert it to a session scoped fixture (be careful to not
pollute it from dependent fixtures, like `config` and clean up the files that
are created inside).

[Jon Sonesen, 2016/10/25 16:29:13]

1. Right now if the bookmark is absent the script fails because if the option is
set it is reasonable to expect that the bookmark exists and if for some reason
it doesn't then it should not proceed building the nightlies, especially since
the bookmarks are platform specific which means if the optionis set but the
bookmark is absent and we use the default it would essentially build a nightly
for the incorrect platform. At least that is my understanding. Because of this I
did not write that test because that is not ideal behavior. Dave probably knows
better if that's how it should be implemented though so I will let him make the
call on that. But if so then I need to make more changes to createNightlies.py
in addition to adding the test.

2.I can do some messing around to see how much code this will take but if it's a
lot then I agree with Dave about doing it as part of a separate ticket.
Although, Wladimir has yet to look this code over as well and he may see
problems. 

[Vasily Kuznetsov, 2016/10/25 16:46:46]

1. I meant "if the abpxxx_bookmark configuration option is absent", sorry, it
was kind of unclear. So if the option is absent, we should check that `master`
bookmark is used. But what you're saying also makes sense and checking that the
script fails if the option is there but the repo doesn't have the bookmark is
also a good idea.

2. I think it should not be too difficult to make the three methods that we're
interested in work and that should be enough of assurance that this change
doesn't break things if they worked before. However, if this change is urgent, I
agree with moving further testing to a separate ticket.

+    assert nightlybuild.bookmark == 'safari'