Merge lp://qastaging/~ralsina/ubuntuone-control-panel/installer-option into lp://qastaging/ubuntuone-control-panel

+1

* I wonder why you added a new method start_from_license instead of redefining setStartId in the QWizard. Any reason not to do that?

* I think that the test_first_page should not be removed, it may need some tweaks so it remains as:

    def test_first_page(self):
        """The first page is the correct one."""
        expected = self.ui.pages[self.ui.signin_page]
        self.assertEqual(self.ui.startId(), expected)

* Can you please fix the indentation of test_start_from_license so the whole 79-columns width are used? Something like (the following may need tweaking if we move to setStartId):

    def test_start_from_license(self):
        """Test the start_from_license method."""
        # Before, we start on sign_in
        assert self.ui.startId() == self.ui.pages[self.ui.signin_page]

        # After calling start_from_license, we start on license_page
        self.ui.start_from_license()
        self.assertEqual(self.ui.startId(), self.ui.pages[self.ui.license_page])

* Why did you need to change the page_id of the buttons dict in UbuntuOneWizardCloudToComputerTestCase, UbuntuOneWizardComputerToCloudTestCase and UbuntuOneWizardSettingsTestCase?

* Why the test_buttons_behavior is skipped for UbuntuOneWizardLicensePage? I see the comment you added but that does not explain to me why you need to redefine/skip it instead of making it pass. Also, why does the NextButton need to be a CommitButton?

* If we're leaving the license 'next' button to be a commit button, we don't need to store the string text of the Next button, so that can be safely removed.

> * I wonder why you added a new method start_from_license instead of redefining
> setStartId in the QWizard. Any reason not to do that?

Moed start_from_license into the controlpanel as suggested.

>
> * I think that the test_first_page should not be removed, it may need some
> tweaks so it remains as:
>
> def test_first_page(self):
> """The first page is the correct one."""
> expected = self.ui.pages[self.ui.signin_page]
> self.assertEqual(self.ui.startId(), expected)

That was already covered by the new test_start_from_license test (had that exact assert).
Readded this test now since start_from_license moved, and moved the test for start_from_license
to control panel tests.

> * Can you please fix the indentation of test_start_from_license so the whole
> 79-columns width are used? Something like (the following may need tweaking if
> we move to setStartId):
>
> def test_start_from_license(self):
> """Test the start_from_license method."""
> # Before, we start on sign_in
> assert self.ui.startId() == self.ui.pages[self.ui.signin_page]
>
> # After calling start_from_license, we start on license_page
> self.ui.start_from_license()
> self.assertEqual(self.ui.startId(),
> self.ui.pages[self.ui.license_page])

Ermmmm I like the way I wrote it? It's wider now anyway because the names are longer.

>
> * Why did you need to change the page_id of the buttons dict in
> UbuntuOneWizardCloudToComputerTestCase, UbuntuOneWizardComputerToCloudTestCase
> and UbuntuOneWizardSettingsTestCase?

Because they didn't work. Those are page IDs, and since now the license page is
added as first page, all those tests failed because they IDs moved.

> * Why the test_buttons_behavior is skipped for UbuntuOneWizardLicensePage? I
> see the comment you added but that does not explain to me why you need to
> redefine/skip it instead of making it pass. Also, why does the NextButton need
> to be a CommitButton?

It has to be a CommitButton so the user can't go back to the license page.
The CommitButton doesn't trigger the same signals as NextButton, and can't find
anywhere in the docs what we may expect there. I changed the test slightly so that
we can check the text of the buttons even if they don't trigger anything by
passing None as signal name.

> * If we're leaving the license 'next' button to be a commit button, we don't
> need to store the string text of the Next button, so that can be safely
> removed.

Can't remove it, causes errors later on. Moved it so it doesn't appear to be part
of the license_page setup.

> > * If we're leaving the license 'next' button to be a commit button, we don't
> > need to store the string text of the Next button, so that can be safely
> > removed.
>
> Can't remove it, causes errors later on. Moved it so it doesn't appear to be
> part
> of the license_page setup.

The line:

self.next_button_text = self.button(self.NextButton).text()

was added by me so we could restore the text in the NextButton in another page, since before we were cutomizing the text in the NextButton for the license page.

As how this branch is, the NextButton text is never changed, so there is no need to keep that hack in place. You can indeed remove it, but you have to removed from all the methods that is being used on (I tried this myself).

Thanks!

> As how this branch is, the NextButton text is never changed, so there is no
> need to keep that hack in place. You can indeed remove it, but you have to
> removed from all the methods that is being used on (I tried this myself).

Ok, removing it everywhere in revno 308

Looks great! Also works as expected.

The attempt to merge lp:~ralsina/ubuntuone-control-panel/installer-option into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

*** Running test suite for ubuntuone/controlpanel ***
ubuntuone.controlpanel.utils.tests.test_utils
  ExceptionHandlingTestCase
    test_exception_to_error_dict ... [OK]
    test_failure_to_error_dict ... [OK]
  GetDataFileTestCase
    test_get_data_file ... [OK]
  GetProjectDirTestCase
    test_get_project_dir_none_exists ... [OK]
    test_get_project_dir_relative ... [OK]
  GetProjectDirWithConstantsTestCase
    test_get_project_dir ... [OK]
    test_get_project_dir_none_exists ... [OK]
    test_get_project_dir_relative ... [OK]
ubuntuone.controlpanel.tests
  TestCase
    runTest ... [OK]
ubuntuone.controlpanel.utils.tests.test_linux
  AutoupdaterTestCase
    test_are_updates_present ... [OK]
    test_perform_update ... [OK]
  DefaultFoldersTestCase
    test_default_folders_bad_encoding ... [OK]
    test_default_folders_empty_file ... [OK]
    test_default_folders_non_ascii ... [OK]
    test_default_folders_not_file ... [OK]
    test_default_folders_only_comments ... [OK]
    test_default_folders_parsed ... [OK]
    test_default_folders_syntax_error ... [OK]
ubuntuone.controlpanel.tests
  TestCase
    runTest ... [OK]
ubuntuone.controlpanel.tests.test_replication_client
  ReplicationsTestCase
    test_exclude ... [OK]
    test_exclude_name_in_exclusions ... [OK]
    test_exclude_name_not_in_replications ... [OK]
    test_get_exclusions ... [OK]
    test_get_replications ... [OK]
    test_no_pairing_record ... [OK]
    test_replicate ... [OK]
    test_replicate_name_not_in_exclusions ... [OK]
    test_replicate_name_not_in_replications ... [OK]
ubuntuone.controlpanel.tests
  TestCase
    runTest ... [OK]
ubuntuone.controlpanel.tests.test_sd_client
  AutoconnectTestCase
    test_disable ... [OK]
    test_disable_throws_an_error ... [...