Automating Tests

[David A. Harding]

Hi,

As I mentioned in a previous post, I want to automated some of the tests
Saīvann and I typically run manually. I think this is an essential
time-saver if we're going to be adding new content to Bitcoin.org on a
regular basis.

To that end, I've written a quick Makefile which performs a few basic
tests. You can see it here:

https://github.com/harding/bitcoin.org/blob/makefile/Makefile

Here's the current output, using -i to ignore (not stop on) errors so
you can see all the currently-detected errors:

$ make -i
Configuration file: /home/harding/bitcoin.org/_config.yml
Deprecation: Auto-regeneration can no longer be set from your configuration file(s). Use the --watch/-w command-line option instead.
Deprecation: The 'server' configuration option is no longer accepted. Use the 'jekyll serve' subcommand to serve your site with WEBrick.
Deprecation: The 'server_port' configuration option has been renamed to 'port'. Please update your config file accordingly.
Source: .
Destination: ./_site
Generating... done.
ERROR:
_site/font/ubuntu-bi-webfont.svg missing corresponding PNG
_site/font/ubuntu-ri-webfont.svg missing corresponding PNG
_site/font/ubuntu-b-webfont.svg missing corresponding PNG
_site/font/ubuntu-r-webfont.svg missing corresponding PNG
_site/img/bubblewarn.svg missing corresponding PNG
_site/img/bubble.svg missing corresponding PNG
_site/img/bubbleinfo.svg missing corresponding PNG
Makefile:55: recipe for target 'ensure-each-svg-has-a-png' failed
make: [ensure-each-svg-has-a-png] Error 1 (ignored)
ERROR:
_site/en/developer-reference.html#getmininginfo not found
Makefile:69: recipe for target 'check-for-missing-anchors' failed
make: [check-for-missing-anchors] Error 1 (ignored)

I can't imagine anyone objects to automated testing, but I'd like to
know whether anyone objects to the approach taken here or has
suggestions for easy-to-implement tests.

(The Makefile is for the entire site, not just devel docs, so expect to
see tests added to help the translators and other contributors.)

If there are no objections, I plan to submit a pull request for the
Makefile after the devel docs gets merged. (The Makefile depends on the
references.md file added in the devel docs pull, so it can't be merged
as-is any earlier.)

Thanks,

-Dave
--
David A. Harding

[Saïvann Carignan]

That is a good idea!

I wonder what's better between having this under _contrib or ./makefile,
as we already have a few maintenance-related scripts in _contrib (I am
not aware if testing builds in makefile is an usual convention, if
that's the case, let's go with a makefile).

Me being a makefile noob, I still tried to provide one improvement, this
one checks for any non-ascii characters in translated urls:

check-for-non-ascii-urls:
## Always check all translated urls don't contain non-ASCII
## characters or spaces.
$S find _translations -name '*.yml' | while read file \
; do cat $$file | sed -n -e '/url:/,$$p' | grep -P
'[^\x00-\x7f]|[a-z\-] [a-z\-]' \
; done | eval $(ERROR_ON_OUTPUT)

However the following code would need to be improved, as it doesn't
report an understandable error message. For the record, this one doesn't
need to run after the website is built.

I've noticed that "HOSTNAME" isn't used elsewhere, perhaps line 8 can be
removed (HOSTNAME=bitcoin.org.test) ?

Saïvann

[David A. Harding]

On Sat, May 17, 2014 at 10:11:47AM -0400, Saīvann Carignan wrote:
> I wonder what's better between having this under _contrib or ./makefile,

> (I am not aware if testing builds in makefile is an usual convention,
> if that's the case, let's go with a makefile).

Makefile-based testing is extremely common... in projects that use
`make` to build themselves. That's not us currently (although we may
want to switch in the future to allow easy conditional execution of
pre-processing or post-processing commands in addition to jekyll.

The two main advantages of having Makefile in the top-level
directory is that (1) you can build and test the site using just `make`
and (2) that all the paths in the Makefile are relative to the top-level
directory instead of being ../ or requiring you run a Makefile in the
_contrib directory from the top-level directory.

(Related to this: I had to add an 'exclude:' directive to the Jekyll
config file so the Makefile didn't get copied into _site, but Jekyll
indicates that the format of exclude has changed over time. It works for
me on 1.5.1, but I need you to test on whatever version gets used to
build the live site.)

> Me being a makefile noob, I still tried to provide one improvement, this
> one checks for any non-ascii characters in translated urls:
>
> check-for-non-ascii-urls:
> ## Always check all translated urls don't contain non-ASCII
> ## characters or spaces.
> $S find _translations -name '*.yml' | while read file \
> ; do cat $$file | sed -n -e '/url:/,$$p' | grep -P
> '[^\x00-\x7f]|[a-z\-] [a-z\-]' \
> ; done | eval $(ERROR_ON_OUTPUT)
>
> However the following code would need to be improved, as it doesn't
> report an understandable error message. For the record, this one doesn't
> need to run after the website is built.

Nice! I'll add it, see if I can get an understandable error message,
and @ mention you in the commit message so GitHub notifies you when I
push it to my branch. Thanks!

> I've noticed that "HOSTNAME" isn't used elsewhere, perhaps line 8 can be
> removed (HOSTNAME=bitcoin.org.test) ?

Whoops. Good catch; I originally had linkchecker code in the Makefile
which required a URL, but I had to remove it after discovering several
bugs with linkchecker (reported to their GitHub issue tracker). I shall
indeed remove that line. Thanks!

[Saïvann Carignan]

Then how about we change "run jekyll" by "run make" in the README?
https://github.com/bitcoin/bitcoin.org#previewing

This doesn't add any dependency, and just provides more useful build
warnings, so I'm all for using it! We could perhaps just keep the
"jekyll --server" trick there for previewing without having a web
server, even though this trick works badly with clean urls.

--

Re makefile copied into _site: that is working just fine here (and in
the event that it gets copied, I think this would be harmless).

--

I've just found a bug, if you run "jekyll build", with recent jekyll
versions, this will build the website in ./build instead of ./_site as
intended. Afterwhile, make doesn't re-build the website:
(make: *** [build] is up to date)

(Although this leads to another issue, previous jekyll versions require
you to run "jekyll build" while recent versions require you to run
"jekyll", I don't know what we can do about this)

$S export LANG=C.UTF-8 ; jekyll build 2>&1 | tee $(JEKYLL_LOG)

$S export LANG=C.UTF-8 ; jekyll 2>&1 | tee $(JEKYLL_LOG)

--

Another bug, you might want to search for svg/png files only in img/ ,
otherwise it reports svg files in font/ , which isn't necessary.

$S find $(SITEDIR)/img -name '*.svg' | while read file \

--

Saïvann

[David A. Harding]

On Sat, May 17, 2014 at 01:14:49PM -0400, Saīvann Carignan wrote:
> Then how about we change "run jekyll" by "run make" in the README?
> https://github.com/bitcoin/bitcoin.org#previewing
>
> This doesn't add any dependency, and just provides more useful build
> warnings, so I'm all for using it!

I don't object, but it does add a dependency on make (and probably the
GNU variant). It also currently adds dependencies on various GNU coreutils
and will likely add dependencies on other software (such as linkchecker)
if we create rules using that software.

I think we should mention that jekyll is all that's required, make will
run various tests it addition to building, and --server can be used for
previews (with the current caveats). If that sounds good, I'll make sure
I put all that in the README for my pull request.

> if you run "jekyll build", with recent jekyll versions, this will

> build the website in ./build [because] previous jekyll versions

> require you to run "jekyll build" while recent versions require you to
> run "jekyll", I don't know what we can do about this)

Isn't that the other way around: old versions use `jekyll`; newer
versions use `jekyll build`? FYI: I'm on v1.5.1 here and it uses `build`
and it looks like jekyll master does too:

https://github.com/jekyll/jekyll/blob/master/lib/jekyll/commands/build.rb

As for a solution, `make` lets you pass variables to your programs in two
ways: on the command line:

make JEKYLL_COMMAND='jekyll build'

Or as an environmental variable, which you can add to your shell:

echo 'export JEKYLL_COMMAND="jekyll build"' >> ~/.bashrc
exec bash
make

I'll add the JEKYLL_COMMAND variable to the Makefile and default it
"jekyll build", which I think is the newer version (so we should probaly
make it the default).

> Another bug, you might want to search for svg/png files only in img/ ,
> otherwise it reports svg files in font/ , which isn't necessary.
>
> $S find $(SITEDIR)/img -name '*.svg' | while read file \

Ah, I didn't know that. Should I only search img/ or should I
just exclude font/ in case somebody puts an image somewhere
outside img/?

Thanks!,

[Saïvann Carignan]

Le 2014-05-17 14:02, David A. Harding a écrit :

> I think we should mention that jekyll is all that's required, make will
> run various tests it addition to building, and --server can be used for
> previews (with the current caveats). If that sounds good, I'll make sure
> I put all that in the README for my pull request.

That sounds good to me

> Isn't that the other way around: old versions use `jekyll`; newer
> versions use `jekyll build`? FYI: I'm on v1.5.1 here and it uses `build`
> and it looks like jekyll master does too:

.. Right, just discovered I'm using a very outdated version..!

> I'll add the JEKYLL_COMMAND variable to the Makefile and default it
> "jekyll build", which I think is the newer version (so we should probaly
> make it the default).

Sounds perfect!

> Ah, I didn't know that. Should I only search img/ or should I
> just exclude font/ in case somebody puts an image somewhere
> outside img/?

I think we should always keep all pictures on img/ , so searching only
img/ is what makes the most sense to me.

Saïvann

[Saïvann Carignan]

Pull request #266 will likely fix the "jekyll vs jekyll build" issue ( but a merge conflict will need to be resolved with @harding branch )

If this branch is merged, I think it's a little cumbersome to type "bundle exec jekyll build" instead of "make" to build the website. So althought this might add some dependencies, I am tempted to say it's still a better solution. Or otherwise if future additional dependencies like linkchecker can cause issues, we could perhaps be using "make" to build the website and "make test" (or similar) to build-test the website?

Saïvann

[David A. Harding]

On Sun, May 18, 2014 at 03:37:20PM -0700, Saīvann Carignan wrote:
> Pull request #266 <https://github.com/bitcoin/bitcoin.org/pull/266> will

> likely fix the "jekyll vs jekyll build" issue ( but a merge conflict will
> need to be resolved with @harding branch )

Cool. I think I'll probably keep the JEKYLL_COMMAND variable around for
anyone who wants to use a custom build command. (That'll probably be
me: as long as there aren't any problems, I'll probably continue using
the version of Jekyll provided by Debian.)

> If this branch is merged, I think it's a little cumbersome to type

> "bundle exec jekyll build" [...] we could perhaps be using "make" to

> build the website and "make test" (or similar) to build-test the
> website?

That's a great idea, and it's consistent with other projects. In my next
push, I'll switch to `make` just for building, `make test` for just
testing, and `make all` for building-and-testing. I'll also see if I can
resolve the conflict (which I guess is just in the README).

[Saïvann Carignan]

>> If this branch is merged, I think it's a little cumbersome to type
>> "bundle exec jekyll build" [...] we could perhaps be using "make" to
>> build the website and "make test" (or similar) to build-test the
>> website?
>
> That's a great idea, and it's consistent with other projects. In my next
> push, I'll switch to `make` just for building, `make test` for just
> testing, and `make all` for building-and-testing. I'll also see if I can
> resolve the conflict (which I guess is just in the README).

Great, thanks!

You probably want to wait for this pull req to be merged, @jm3 is often
replacing his commit :) .

Saïvann