Skip to content

Added test about status.php#29265

Merged
PVince81 merged 4 commits intomasterfrom
int-test-check-statusphp
Oct 19, 2017
Merged

Added test about status.php#29265
PVince81 merged 4 commits intomasterfrom
int-test-check-statusphp

Conversation

@SergioBertolinSG
Copy link
Contributor

This is a test for #29228

it currently fails, requires #29261 .

@SergioBertolinSG SergioBertolinSG changed the title Added test about status.php [WIP] Added test about status.php Oct 17, 2017
When requesting status.php
Then the json responded should match with
"""
{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"10.0.3.3","versionstring":"10.0.3","edition":"Community","productname":"ownCloud"}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail each time the ownCloud version number is bumped.
I did a similar sort of test for a version number in returned data in https://github.com/owncloud/core/pull/29230/files - look for "attribute value should be a valid version string" step. If you extract version and versionstring from the JSON then you can use version_compare() to discover if the data is a "reasonable valid" version number representation.

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 was thinking about checking the actual values by using occ.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you could do that also - get those current version strings by some other way and make sure they are the same.

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 great

@SergioBertolinSG
Copy link
Contributor Author

Ok some changes, now the version and versionstrings won't require to be changed with any newer version.

To achieve that I've refactored CommandLine trait to include all the code from CommandLineContext, removing it.

There is a variable required called ocPath, and I've added it to the constructor. This will affect all the apps, and will require to add that variable in customgroups and guests at least.

@SergioBertolinSG SergioBertolinSG changed the title [WIP] Added test about status.php Added test about status.php Oct 17, 2017
@SergioBertolinSG
Copy link
Contributor Author

Please rereview @PVince81 @phil-davis

@codecov
Copy link

codecov bot commented Oct 17, 2017

Codecov Report

Merging #29265 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #29265   +/-   ##
=========================================
  Coverage     59.56%   59.56%           
  Complexity    17156    17156           
=========================================
  Files          1029     1029           
  Lines         57211    57211           
=========================================
  Hits          34076    34076           
  Misses        23135    23135

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6b7738...13bb778. Read the comment docs.

@SergioBertolinSG SergioBertolinSG force-pushed the int-test-check-statusphp branch from 29fcc80 to b85a147 Compare October 18, 2017 07:33
Copy link
Contributor

Choose a reason for hiding this comment

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

having X in the expected version here doesn't feel right.

another approach could be to check keys individually:
"the response contains a key "installed" with value true"

in this case the version string would be checked with only:
"the response contains a key "versionString".

Also note that we have config switches to disable exposing the version, so we should also have scenarios for these

Copy link
Contributor Author

@SergioBertolinSG SergioBertolinSG Oct 18, 2017

Choose a reason for hiding this comment

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

Those X are changed with the current version of the server before comparing.

It gets their values using occ and then changes x.x.x.x for 10.0.3.3 for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what I mean with "versions fixed " in the step name.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know, but to someone non-technical reading the scenarios this is not self-explanatory.

If you do not want to change this to the above proposal, then I suggest changing the strings to "$CURRENT_VERSION_STRING" which most will understand that a substitution will happen there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, that sounds good.

@PVince81
Copy link
Contributor

@SergioBertolinSG please backport

@phil-davis
Copy link
Contributor

Backport to stable10 is in PR #29296

@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