Move all doc-links to a collected space, Add command to verify all available doc links

[AlexAndBear]

Description

All available doc links can now be tested and verified via make test-doc-links. It is also possible to
specify the ownCloud version to look for in the docs via make test-doc-links DOC_LINK_VERSION=10.7.
It defaults to the ownCloud version of the current installation.

Related Issue

• Links with https://github.com/owncloud/enterprise/issues/4671

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised: Document command to verify all available doc links docs#3900
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[AlexAndBear]

@phil-davis not quite sure how to handle the tests, run them for every server or just once ?

[phil-davis]

https://github.com/owncloud/core/tree/docs-testing
"This branch is 1 commit ahead, 25 commits behind master."

If you rebase, then you won't need to have all the diffs that are here.

[AlexAndBear]

@phil-davis ok rebased, 1 diff less :)

[ownclouders]

💥 Acceptance tests pipeline apiFederationToShares1-latest-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/31356/114/1

[JammingBen]

@phil-davis We added a drone step doclinks which basically calls this unit test.

In case of failure we want the pipeline to continue while posting a comment on Github. We already achieved that the error gets ignored via 'failure': 'ignore'. The problem: the Github comment wont be posted because it listens to the failure event, which we suppress. Any idea? :/

Another thing to mention: The pipeline runs based on daily master, right? Which means after #39048 has been merged, the doc pipeline always fails until the docs for 10.9 are available.

[phil-davis]

1. I don't know of any way around this problem in drone with GitHub comment. I am trying to do a similar thing in [tests-only] [full-ci] Report passed-on-retry to RocketChat web#5409 - that is for reporting scenarios that passed-on-retry to RocketChat. In that case I don't fail the acceptance test step, I just write some text to a file. In the later notify-passedOnRetry I want to only actually post to RocketChat if the file has some text in it. The plugins/slack image is not smart enough to do a "if only".

Here's an idea - in web we always run the github-comment step and it has a command that only does something if text for a comment exists.

        "commands": [
            "if [ -s /var/www/owncloud/web/comments.file ]; then echo '%s' | cat - comments.file > temp && mv temp comments.file && /bin/drone-github-comment; fi" % prefix,
       ],

Keep ignoring failure of the tests, but write something in comments.file if a comment is needed. Then githubComment can post the comment only if there is one.

2. I don't really know what should be the "workflow" for testing against some external thing that we know will not exist until a few days before the release. The test could have a way to test against the docs links for the "previous release"? And then we have to update what is "previous release" when the new release is really getting ready. It will also be painful if the docs links change or there are new links - those will definitely not exist in an old release.

[AlexAndBear]

@phil-davis IHMO it is crucial, on the one hand we want to make sure that all links to docs are correct, on the other hand we don't want to do hard depending checks. As @xoxys required some tests, he knows best what he wants to archive.

If we suspect owncloud is a ecosystem, and docs is a part of it, I think Integration tests need to cover it, too...

[phil-davis]

Note: https://github.com/owncloud/core/blob/master/tests/acceptance/features/bootstrap/WebUIHelpAndTipsContext.php has test code that checks that the links displayed in the Help & Tips on the webUI are "valid" - that they have the expected URL.

https://github.com/owncloud/core/blob/master/tests/acceptance/features/webUIAdminSettings/helpAndTips.feature has the test scenario.

That code does not actually go out and check that the links respond on the "big bad internet" in real-time, because we had the same problem as we have here. The real docs site might be down, might not be reachable from the test machine, or the next version is in development (e.g. 10.9) and there are no docs for that yet.

[JammingBen]

I don't really know what should be the "workflow" for testing against some external thing that we know will not exist until a few days before the release. The test could have a way to test against the docs links for the "previous release"? And then we have to update what is "previous release" when the new release is really getting ready. It will also be painful if the docs links change or there are new links - those will definitely not exist in an old release.

Yep, I'm afraid there is currently no nice way to handle this.

So, correct me if I'm wrong, but the main goal here is to have a valid list of doc identifiers which are used in OC. And to keep all parties informed if that list changes somehow.

This PR introduces such a list: https://github.com/owncloud/core/pull/39026/files#diff-4392b98f2438a8b2698467bda6228bed8a258ec62c83a6fa15e97c0ad1781089. Wouldn't it be sufficient to keep track of this list? Meaning, if some future PR changes something by adding, editing or removing anything, the doc team gets noticed. @xoxys @janackermann

[xoxys]

Well, this list might be still bad for automated testing. If we can not implement this in the test suite of oC core (and I agree on the downsides/blockers) I would like to have the possibility to run a simple bash loop like this:

URLS="admin-email2 admin-backup"
for URL in $URLS; do
    curl -o /dev/null --silent --head --write-out '%{http_code}\n' https://doc.owncloud.com/server/10.7/go.php?to=${URL};
done

# result
404
200

We need a way to run a simple test manually at any time (or a simple way to add it to QA), copying IDs from the code might not be the best way :) Do you know a way to get it out of the code automatically? Or instead of bash, can we have a php -f <run-command> ... to test all doc links manually?

[JammingBen]

Do you know a way to get it out of the code automatically? Or instead of bash, can we have a php -f ... to test all doc links manually?

Sure, we can implement a PHP script that gathers those url identifiers from the list and validates them via curl request. The script itself would be located somewhere in the core repo (make it callable via Makefile?). I suggest to pass the OC version via optional param, which defaults to the latest version.

[xoxys]

Sounds good to me, this way it can be tested easily any time. @jnweiger would it make sense for you to add it as a QA step to run such a test script and report broken links to sysadmin/docs team?

[phil-davis]

make it callable via Makefile

Good idea. We have targets like make test-php-style, so something like make test-doc-links ?

[JammingBen]

I updated the PR and implemented make test-doc-links. You can also specify the OC version to test via make test-doc-links DOC_LINK_VERSION=10.7. Feel free to try.

[IljaN]

LGTM

[phil-davis]

$ make test-doc-links DOC_LINK_VERSION="10.9"
All doclinks are valid!

I did some output to debug. It checks links like https://doc.owncloud.com/server/10.9/go.php?to=admin-email

That link "works" in the sense that it returns 200 "OK" and "effectiveUrl" = "https://doc.owncloud.com/server/10.9/admin_manual/configuration/server/email_configuration.html"

But https://doc.owncloud.com/server/10.9/admin_manual/configuration/server/email_configuration.html responds with "404
Oops! You're lost. "

IMO the test needs to follow the effectiveUrl and see if that really works.

Here is an example of a response object that PHP dumped for me:

object(OC\Http\Client\Response)#990 (2) {
  ["response":"OC\Http\Client\Response":private]=>
  object(GuzzleHttp\Message\Response)#987 (7) {
    ["reasonPhrase":"GuzzleHttp\Message\Response":private]=>
    string(2) "OK"
    ["statusCode":"GuzzleHttp\Message\Response":private]=>
    int(200)
    ["effectiveUrl":"GuzzleHttp\Message\Response":private]=>
    string(95) "https://doc.owncloud.com/server/10.9/admin_manual/configuration/server/email_configuration.html"
    ["headers":"GuzzleHttp\Message\AbstractMessage":private]=>
    array(2) {
      ["date"]=>
      array(1) {
        [0]=>
        string(29) "Tue, 27 Jul 2021 12:06:53 GMT"
      }
      ["transfer-encoding"]=>
      array(1) {
        [0]=>
        string(7) "chunked"
      }
    }
    ["headerNames":"GuzzleHttp\Message\AbstractMessage":private]=>
    array(2) {
      ["date"]=>
      string(4) "Date"
      ["transfer-encoding"]=>
      string(17) "Transfer-Encoding"
    }
    ["body":"GuzzleHttp\Message\AbstractMessage":private]=>
    object(GuzzleHttp\Stream\Stream)#1000 (7) {
      ["stream":"GuzzleHttp\Stream\Stream":private]=>
      resource(538) of type (stream)
      ["size":"GuzzleHttp\Stream\Stream":private]=>
      NULL
      ["seekable":"GuzzleHttp\Stream\Stream":private]=>
      bool(true)
      ["readable":"GuzzleHttp\Stream\Stream":private]=>
      bool(true)
      ["writable":"GuzzleHttp\Stream\Stream":private]=>
      bool(true)
      ["uri":"GuzzleHttp\Stream\Stream":private]=>
      string(10) "php://temp"
      ["customMetadata":"GuzzleHttp\Stream\Stream":private]=>
      array(0) {
      }
    }
    ["protocolVersion":"GuzzleHttp\Message\AbstractMessage":private]=>
    string(3) "1.1"
  }
  ["stream":"OC\Http\Client\Response":private]=>
  bool(false)
}

[JammingBen]

Good catch!

IMO the test needs to follow the effectiveUrl and see if that really works.

effectiveUrl also returns 200 as status code in this case. https://doc.owncloud.com/server/10.9/go.php?to=admin-email should return a proper 404 code. Just like https://doc.owncloud.com/server/10.9/go.php?to=admin-email2 does.

[JammingBen]

@phil-davis Could you please try again? Should work now

[phil-davis]

$ make test-doc-links DOC_LINK_VERSION="10.9"
php tests/docs/DocLinksTest.php 10.9
Doclink for 'admin-email' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-email
Doclink for 'admin-backup' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-backup
Doclink for 'admin-config' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-config
Doclink for 'admin-sharing' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-sharing
Doclink for 'admin-php-fpm' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-php-fpm
Doclink for 'admin-install' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-install
Doclink for 'admin-security' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-security
Doclink for 'admin-logfiles' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-logfiles
Doclink for 'admin-encryption' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-encryption
Doclink for 'admin-performance' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-performance
Doclink for 'admin-cli-upgrade' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-cli-upgrade
Doclink for 'admin-db-conversion' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-db-conversion
Doclink for 'admin-source_install' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-source_install
Doclink for 'admin-background-jobs' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-background-jobs
Doclink for 'admin-untrusted-domain' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-untrusted-domain
Doclink for 'admin-sharing-federated' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-sharing-federated
Doclink for 'admin-transactional-locking' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=admin-transactional-locking
Doclink for 'user-webdav' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=user-webdav
Doclink for 'developer-theming' failed with status code 404: https://doc.owncloud.com/server/10.9/go.php?to=developer-theming

$ make test-doc-links DOC_LINK_VERSION="10.7"
php tests/docs/DocLinksTest.php 10.7
All doclinks are valid!

$ make test-doc-links
php tests/docs/DocLinksTest.php 
All doclinks are valid!

works

[AlexAndBear]

Do we need this command in docs, for example dev docs ?

[phil-davis]

Do we need this command in docs, for example dev docs ?

yes, it would be good to mention it somewhere.

docs issue owncloud/docs#3900 has been created.

[ownclouders]

💥 Acceptance tests pipeline apiSharePublicLink2-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/31571/77/1

[AlexAndBear]

Good to merge ?

[phil-davis]

Good to merge ?

Yes, please merge when CI is green.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

85.7% Coverage
0.0% Duplication