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

Update the minimum desktop version to 2.2.4 #28017

Merged
merged 1 commit into from
Jul 4, 2017

Conversation

settermjd
Copy link
Contributor

@settermjd settermjd commented May 26, 2017

As per the request in #27978, this PR was created to resolve it, setting the minimum ownCloud client version to 2.2.4.

Relates To

@@ -77,7 +77,7 @@ public function testBeforeHandlerException($userAgent) {
$this->config
->expects($this->once())
->method('getSystemValue')
->with('minimum.supported.desktop.version', '2.0.0')
->with('minimum.supported.desktop.version', '2.2.4')
->will($this->returnValue('1.7.0'));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if this should return 2.2.4 instead of 1.7.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have thought this would be fine, as it's testing a before handler exception.

@@ -112,7 +112,7 @@ public function testBeforeHandlerSuccess($userAgent) {
$this->config
->expects($this->once())
->method('getSystemValue')
->with('minimum.supported.desktop.version', '2.0.0')
->with('minimum.supported.desktop.version', '2.2.4')
->will($this->returnValue('1.7.0'));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if this should return 2.2.4 instead of 1.7.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well said. Funny that the tests didn't fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the value "2.2.4" is not relevant to the test itself, but the return value is.

The value "2.2.4" is only there to make sure that the function getSystemValue was called with the correct default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reset the original values and the tests failed. After that, I updated the values in this case, both to be set to 2.2.4, and also updated the test data in newAndAlternateDesktopClientProvider, and the tests now work.

This commit sets the minimum ownCloud client version to
2.2.4. This fixes #27978.
@settermjd settermjd force-pushed the change-minimum-desktop-version-to-2.2.4 branch from 961b343 to bd80eed Compare May 31, 2017 10:59
@settermjd
Copy link
Contributor Author

@kdslkdsaldsal - ready to go now?

@ghost
Copy link

ghost commented May 31, 2017

@settermjd Sorry, i can't say something about the tests as i don't have any background knowledge about these.

@PVince81
Copy link
Contributor

Since you didn't change the semantics of the tests and their results I think this change is ok.
The logic that was tested here was whether comparing the version would make the plugin throw an exception or not with some sets of values.

👍

@PVince81
Copy link
Contributor

please rebase, I do want to see CI pass here

@settermjd
Copy link
Contributor Author

Rebased and pushed. Let's see what we can do.

@phil-davis
Copy link
Contributor

phil-davis commented May 31, 2017

Today various Travis jobs seem to be failing "randomly" just trying to "make" the code:

The command "make" failed and exited with 2

regardless of if they are the UI tests or other Travis jobs. I restarted the ones that failed - cross your fingers.
And the Travis jobs passed 2nd time.

@PVince81 PVince81 merged commit 691f951 into master Jul 4, 2017
@PVince81 PVince81 deleted the change-minimum-desktop-version-to-2.2.4 branch July 4, 2017 10:05
@PVince81
Copy link
Contributor

PVince81 commented Jul 4, 2017

@settermjd please backport to stable10

@settermjd
Copy link
Contributor Author

settermjd commented Jul 31, 2017

Backport initiated, @PVince81, in #28540.

@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants