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

On 06/06/2012 22:18, Matthieu Moy wrote:

Simon Cathebras[off-list ref]  writes:

quoted

From: Charles Roussel<redacted>

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)

Understood, it will be corrected in the next patch.

quoted

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

s/du/to/

quoted

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).

Do you mean that we should split the third patch into two patches ? For 
instance::
     Patch 3/4: tests for git pull
     Patch 4/4: tests for git push

quoted

+# 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?

Deleted.

quoted

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

Why / in front of $GIT_BUILD_DIR/ ?

Good point, we didn't noticed it ...

quoted

+        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?

Yes, we haven't realised it wasn't necessary.
We have corrected other errors like this one in our tests.

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.

Done, and replace within the existing code.

quoted

+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/

quoted

+	git_diff_directories mw_dir ref_page&&

functions in tests are usually prefixed with test_ instead.

Fair enough. We have changed the function's name. Should we add such 
prefix on functions like wiki_reset or wiki_delete page ?

quoted

+# 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.

Test is now correct.

Thanks for the advices ;).

-- 
CATHEBRAS Simon

2A-ENSIMAG

Filière Ingéniérie des Systèmes d'Information
Membre Bug-Buster