Re: [PATCH 3/3] Tests file for git-remote-mediawiki

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Simon Cathebras <simon.cathebras@ensimag.imag.fr> writes:

> From: Charles Roussel <charles.roussel@ensimag.fr>
>
> Those scripts test the functions of git-remote-mediawiki.
> t9360: test cases for git clone. Including tests of cloning just a category, just a precise set of page and a classical use of clone on the whole wiki.

Avoid long lines in code and commit messages (80 columns max)

> In addition, this file provide now some fonction du manipulate sections on wiki.

s/du/to/

> t9361: test cases for git pull (add page, edit page, delete page) and git push (add file, edit file, delete file).

When commis messages start looking like an enumeration, it usually means
that either you read the GNU recommandation for ChangeLogs too much, or
that you should split your commit (not mandatory here I think, but short
patches are easier to review).

> +# tests for git-remote-mediawiki
> +
> +test_description='Test the Git Mediawiki remote helper: git clone'

Why do you need a comment if you have the test_description right below?

> +if [ ! -f /$GIT_BUILD_DIR/git-remote-mediawiki ];

Why / in front of $GIT_BUILD_DIR/ ?

> +        test_expect_code 0 ls mw_dir | wc -l | grep 1 &&
> +        test_expect_code 0 test -e mw_dir/Main_Page.mw &&

Why "test_expect_code 0"? You already have && right?

Doesn't a directory containing 10 files pass the tests?

You probably want a helper function test_contains_N_files <dir> <N> that
does test `ls mw_dir | wc -l` -eq 1 as you did below, but may give a
diagnosis when the test fails.

> +test_expect_success 'git clone only create Main_Page.mw with a wiki with no other pages ' '
> +        wiki_reset &&
> +        wiki_editpage foo "this page must be delete before the clone" false &&

s/delete/deleted/

> +	git_diff_directories mw_dir ref_page &&

functions in tests are usually prefixed with test_ instead.

> +# clone a wiki after a page has been added then edited once
> +# check that the content is correct

It's not sufficient. You should check also that the history is correct.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/