Re: [PATCH v2] travis-ci: build documentation

[Junio C Hamano]

Lars Schneider  writes:

>> This could be less of maintenance if we'd check with a "larger as" operator
>> such as
>> 
>>test_file_count_more_than html 200
>> 
>> using an arbitrary slightly smaller number.
>
> Well, I was thinking about testing against something like 
> $(find . -type f -name "git*.txt" | wc -l) but it the end
> all of this is not really meaningful I think...

Either is too much, I would say--I have too much faith in the exit
status from "make doc", I guess.

What would _REALLY_ be nice is a check that lets us catch an error
like this deliberate breakage:

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 4b0318e..a684f2d 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -560,4 +560,4 @@ endif::git-format-patch[]
Do not show any source or destination prefix.

 For more detailed explanation on these common options, see also
-linkgit:gitdiffcore[7].
+linkgit:gitdiffnore[7].

We just get a dangling link in the result without an error exit from
"make doc"; neither "test -s" nor "size is about what we expect"
would catch such a breakage, though.

Other things that might be of interest are

make check-builtins
make check-docs

but I am not sure if the latter built target is up to date (it has a
whiltelist that needs to stay current).  We rarely add new commands
these days, so it is easy to forget what these build targets try to
check, which makes them good candidates to be thrown into the set of
automated tests.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Lars Schneider]

On 29 Apr 2016, at 19:27, Stefan Beller  wrote:

> On Fri, Apr 29, 2016 at 5:21 AM, Matthieu Moy
>  wrote:
>> Jeff King  writes:
>> 
>>> On Fri, Apr 29, 2016 at 11:35:34AM +0200, larsxschnei...@gmail.com wrote:
>>> 
 +# The follow numbers need to be adjusted when new documentation is added.
 +test_file_count html 233
 +test_file_count xml 171
 +test_file_count 1 152
>>> 
>>> This seems like it will be really flaky and a pain in the future. I'm
>>> not really sure what it's accomplishing, either. The earlier steps would
>>> complain if something failed to render, wouldn't they? At some point we
>>> have to have some faith in "make doc".
>> 
>> I agree. My proposal to check for a handful of generated files was just
>> because this extra paranoia was almost free (just 3 lines of code that
>> won't need particular maintenance).
>> 
>> In this case, I'm afraid the maintenance cost is much bigger than the
>> expected benefits.
> 
> So you proposed to check a handful files for its exact content?
> 
> This could be less of maintenance if we'd check with a "larger as" operator
> such as
> 
>test_file_count_more_than html 200
> 
> using an arbitrary slightly smaller number.

Well, I was thinking about testing against something like 
$(find . -type f -name "git*.txt" | wc -l) but it the end
all of this is not really meaningful I think...

- Lars


> 
> 
>> 
>> --
>> Matthieu Moy
>> http://www-verimag.imag.fr/~moy/
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Matthieu Moy]

Stefan Beller  writes:

> On Fri, Apr 29, 2016 at 5:21 AM, Matthieu Moy
>  wrote:
>> Jeff King  writes:
>>
>>> On Fri, Apr 29, 2016 at 11:35:34AM +0200, larsxschnei...@gmail.com wrote:
>>>
 +# The follow numbers need to be adjusted when new documentation is added.
 +test_file_count html 233
 +test_file_count xml 171
 +test_file_count 1 152
>>>
>>> This seems like it will be really flaky and a pain in the future. I'm
>>> not really sure what it's accomplishing, either. The earlier steps would
>>> complain if something failed to render, wouldn't they? At some point we
>>> have to have some faith in "make doc".
>>
>> I agree. My proposal to check for a handful of generated files was just
>> because this extra paranoia was almost free (just 3 lines of code that
>> won't need particular maintenance).
>>
>> In this case, I'm afraid the maintenance cost is much bigger than the
>> expected benefits.
>
> So you proposed to check a handful files for its exact content?

No, just to check that the files exist and are non-empty, i.e. the "test
-s" part of Lars' patch.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Stefan Beller]

On Fri, Apr 29, 2016 at 5:21 AM, Matthieu Moy
 wrote:
> Jeff King  writes:
>
>> On Fri, Apr 29, 2016 at 11:35:34AM +0200, larsxschnei...@gmail.com wrote:
>>
>>> +# The follow numbers need to be adjusted when new documentation is added.
>>> +test_file_count html 233
>>> +test_file_count xml 171
>>> +test_file_count 1 152
>>
>> This seems like it will be really flaky and a pain in the future. I'm
>> not really sure what it's accomplishing, either. The earlier steps would
>> complain if something failed to render, wouldn't they? At some point we
>> have to have some faith in "make doc".
>
> I agree. My proposal to check for a handful of generated files was just
> because this extra paranoia was almost free (just 3 lines of code that
> won't need particular maintenance).
>
> In this case, I'm afraid the maintenance cost is much bigger than the
> expected benefits.

So you proposed to check a handful files for its exact content?

This could be less of maintenance if we'd check with a "larger as" operator
such as

test_file_count_more_than html 200

using an arbitrary slightly smaller number.


>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Matthieu Moy]

larsxschnei...@gmail.com writes:

> +  before_install:
> +  before_script: make doc
> +  script: ci/test-documentation.sh

If you are to re-roll, I think before_script and script should be
merged, i.e. just write

script: ci/test-documentation.sh

and have ci/test-documentation.sh be:

#!/bin/sh

set -e

make doc
test -s Documentation/git.html
test -s Documentation/git.xml
test -s Documentation/git.1

In the perspective of using another CI tool, everything within ci/ is
potentially shared between systems so I'd tend to minimize the content
of .travis.yml in favor of ci/*

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Jeff King]

On Fri, Apr 29, 2016 at 04:22:05PM +0200, Lars Schneider wrote:

> >>> +# The follow numbers need to be adjusted when new documentation is added.
> >>> +test_file_count html 233
> >>> +test_file_count xml 171
> >>> +test_file_count 1 152
> [...]
> I agree, too. I wasn't sure about this check. That's why I added
> the little comment above to point out the problem.
> 
> Should I reroll?

IMHO, yes.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Lars Schneider]

> On 29 Apr 2016, at 14:21, Matthieu Moy  wrote:
> 
> Jeff King  writes:
> 
>> On Fri, Apr 29, 2016 at 11:35:34AM +0200, larsxschnei...@gmail.com wrote:
>> 
>>> +# The follow numbers need to be adjusted when new documentation is added.
>>> +test_file_count html 233
>>> +test_file_count xml 171
>>> +test_file_count 1 152
>> 
>> This seems like it will be really flaky and a pain in the future. I'm
>> not really sure what it's accomplishing, either. The earlier steps would
>> complain if something failed to render, wouldn't they? At some point we
>> have to have some faith in "make doc".
> 
> I agree. My proposal to check for a handful of generated files was just
> because this extra paranoia was almost free (just 3 lines of code that
> won't need particular maintenance).
> 
> In this case, I'm afraid the maintenance cost is much bigger than the
> expected benefits.

I agree, too. I wasn't sure about this check. That's why I added
the little comment above to point out the problem.

Should I reroll?

Thanks,
Lars--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Matthieu Moy]

Jeff King  writes:

> On Fri, Apr 29, 2016 at 11:35:34AM +0200, larsxschnei...@gmail.com wrote:
>
>> +# The follow numbers need to be adjusted when new documentation is added.
>> +test_file_count html 233
>> +test_file_count xml 171
>> +test_file_count 1 152
>
> This seems like it will be really flaky and a pain in the future. I'm
> not really sure what it's accomplishing, either. The earlier steps would
> complain if something failed to render, wouldn't they? At some point we
> have to have some faith in "make doc".

I agree. My proposal to check for a handful of generated files was just
because this extra paranoia was almost free (just 3 lines of code that
won't need particular maintenance).

In this case, I'm afraid the maintenance cost is much bigger than the
expected benefits.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] travis-ci: build documentation

[Jeff King]

On Fri, Apr 29, 2016 at 11:35:34AM +0200, larsxschnei...@gmail.com wrote:

> +# The follow numbers need to be adjusted when new documentation is added.
> +test_file_count html 233
> +test_file_count xml 171
> +test_file_count 1 152

This seems like it will be really flaky and a pain in the future. I'm
not really sure what it's accomplishing, either. The earlier steps would
complain if something failed to render, wouldn't they? At some point we
have to have some faith in "make doc".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] travis-ci: build documentation

[larsxschneider]

From: Lars Schneider 

Run "make doc" as separate Travis CI build job to check if all
documentation can be built without errors.

Signed-off-by: Lars Schneider 
---

diff to v1:
* add quick sanity check to documentation results (thanks Matthieu)
* fix typo in commits message (thanks Stefan)
* move 'make doc' into seperate Travis CI build job (thanks Peff)
* started to move CI helper scripts to /ci (thanks Matthieu & Junio)

I really like the idea of the "/ci" directory. Over time I plan to
migrate all non Travis-CI related code from .travis.yml to this directory.
This would ease the transition to or parallel execution with another CI
system (e.g. GitLab CI to test Git on Windows).

Thanks,
Lars


 .travis.yml  | 15 +++
 ci/test-documentation.sh | 23 +++
 2 files changed, 38 insertions(+)
 create mode 100755 ci/test-documentation.sh

diff --git a/.travis.yml b/.travis.yml
index 78e433b..9f71d23 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -32,6 +32,21 @@ env:
 # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI 
OS X
 - GIT_SKIP_TESTS="t9810 t9816"

+matrix:
+  include:
+- env: Documentation
+  os: linux
+  compiler: clang
+  addons:
+apt:
+  packages:
+  - asciidoc
+  - xmlto
+  before_install:
+  before_script: make doc
+  script: ci/test-documentation.sh
+  after_failure:
+
 before_install:
   - >
 case "${TRAVIS_OS_NAME:-linux}" in
diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
new file mode 100755
index 000..329ff4b
--- /dev/null
+++ b/ci/test-documentation.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+#
+# Perform a quick sanity check on documentation generated with 'make doc'.
+#
+
+set -e
+
+test_file_count () {
+SUFFIX=$1
+EXPECTED_COUNT=$2
+ACTUAL_COUNT=$(find Documentation -type f -name "*.$SUFFIX" | wc -l)
+echo "$ACTUAL_COUNT *.$SUFFIX files found. $EXPECTED_COUNT expected."
+test $ACTUAL_COUNT -eq $EXPECTED_COUNT
+}
+
+test -s Documentation/git.html
+test -s Documentation/git.xml
+test -s Documentation/git.1
+
+# The follow numbers need to be adjusted when new documentation is added.
+test_file_count html 233
+test_file_count xml 171
+test_file_count 1 152
--
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html