Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: add naive travis config file #50

Merged
merged 9 commits into from
Nov 22, 2017
Merged

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Nov 20, 2017

Closes: #48

@anarcat
Copy link
Contributor Author

anarcat commented Nov 20, 2017

obviously, the whole test suite is broken now because it depends on the Git test suite. i've tried to tackle this by making the TEST_DIRECTORY customizable and checking out the git source in the build, we'll see how that goes.

@anarcat anarcat force-pushed the travis branch 4 times, most recently from 7b01f01 to e9c27ab Compare November 20, 2017 19:34
@anarcat
Copy link
Contributor Author

anarcat commented Nov 20, 2017

raaaah... i give up: git is bending over backwards to keep us from running tests with its test suite.

i'm thinking more and more the only way to get out of this shit hole is to repackage this using CPAN - see #18.

@moy any better brilliant ideas to fix the test suite so it works without a built git tree?

i mean obviously we could build all of git inside Travis, but that seems such a waste that I didn't even consider it...

@moy
Copy link
Collaborator

moy commented Nov 21, 2017

Ideally, we could port the testsuite to Sharness, which is essentially Git's testsuite but made standalone. But that's probably a non-negligible effort (not all features of the Git testsuite are available, some are ported from time to time).

It may be simpler to hack the Travis build to move the Git-Mediawiki repository to actually be in Git's contrib/mw-to-git subdirectory for the time of the build. I don't think you'd need to build Git itself, just to have Git-Mediawiki in the right subdirectory. But I may be wrong ...

@anarcat
Copy link
Contributor Author

anarcat commented Nov 21, 2017

as you can see in the travis logs, the git test suite expects git to be fully built. that's where i gave up.

i wonder if this shouldn't be part of the move to CPAN in #18 ... perl has its own test harness and it may be more logical to port to that instead, no?

@moy
Copy link
Collaborator

moy commented Nov 21, 2017

Porting to a perl test-harness is an option, but the advantage of Sharness is that the modifications are much smaller (it may just work out of the box without modifying the scripts). Otherwise, you have 2000 LOC of shell to rewrite to Perl, that won't be fun.

Another theoretical advantage of using Sharness/Git's testsuite is that it is well known from Git contributors. In practice it's probably only theoretical since we haven't seen contributions from the Git community for years.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

i agree - it is probably simpler to use sharness. but for the record, the Perl test suite is actually the basis of the "TAP" protocol that sharness and Git uses, so the output is basically identical.

converting shell scripts to perl is also quite easy to do - the problem is that the API is actually different, strangely enough. whereas Sharness/Git do:

test_expect_success 'sucess' true

perl does:

ok(1, 'success')

so it would require porting. i'll look into sharness.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

so porting to sharness seems fairly straightforward. i added it as a submodule here and it basically just works.

now the problem is mediawiki doesn't work. On debian stable, php5 doesn't exist anymore, so I changed the dependencies to "php" which means whatever php version is in debian now, which is PHP 7, which Mediawiki 1.21 fails with.

So I updated to Mediawiki 1.27. and now all sorts of shit break. the installer, for one, so i fixed that. but now "login" doesn't work somehow, in the mediawiki API:

File install-wiki/wikidb.sqlite is set in /tmp
Use of the encoding pragma is deprecated at /home/anarcat/src/Git-Mediawiki/t/test-gitmw.pl line 27.
getpage: login failed at /home/anarcat/src/Git-Mediawiki/t/test-gitmw.pl line 65.
not ok 2 - Git clone creates the expected git log with one file
#	
#		wiki_reset &&
#		wiki_editpage foo "this is not important" false -c cat -s "this must be the same" &&
#		git clone mediawiki::http://localhost:1234/wiki mw_dir_1 &&
#		(
#			cd mw_dir_1 &&
#			git log --format=%s HEAD^..HEAD >log.tmp
#		) &&
#		echo "this must be the same" >msg.tmp &&
#		diff -b mw_dir_1/log.tmp msg.tmp

now i can login to the mediawiki fine by hand, in a web browser. so it seems something is broken in the API.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

and now of course travis doesn't have php7 - so let's try again without that crap.

@anarcat anarcat force-pushed the travis branch 2 times, most recently from 444a5d9 to 0fb8bc3 Compare November 22, 2017 16:19
@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

okay, we are in a good state now - tests almost pass: 3 tests fail because the log output is not what is expected. not sure what is going on...

@moy can you look at the travis log here and tell me if you can figure out what's wrong with the tests now?

https://travis-ci.org/Git-Mediawiki/Git-Mediawiki/builds/305904207

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

and obviously, we're not up to date in PHP or Mediawiki, but that's another issue we can fix seperately. it does mean it's much harder for me to fix this because i run the tests locally...

@moy
Copy link
Collaborator

moy commented Nov 22, 2017

Good job!

The failure is strange: sometimes, git log gives an empty output. I first thought it was because we restricted it to a file (git log <file>.mw) and the file may have the wrong content, but there's one failure for a git log without a path argument.

You may try adding git ls-files here and there in the failing test to check for actual files tracked, and git log to see what is its output.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

@moy i added some debug info, but frankly i'm just shooting in the dark here...

https://travis-ci.org/Git-Mediawiki/Git-Mediawiki/builds/305923876

it sure looks like it's importing the history twice... maybe we're reproducing #29?

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

i marked those tests as "expected failures" to try and resolve this. if you can look into those, I would be very grateful because I feel I'm just pulling at straws here and I'd like to get something working here...

@moy
Copy link
Collaborator

moy commented Nov 22, 2017

I was about to suggest the same as what you just did: mark tests as failures (you may add a new issue so these expect_failure are not forgotten), and then merge this. At least it gives some regression testing for other PRs.

Unfortunately I don't understand what's going on either, and I don't have time to investigate more (I won't in the next few days/weeks at least).

@anarcat anarcat force-pushed the travis branch 2 times, most recently from 7f55cf4 to 0fee6f9 Compare November 22, 2017 18:01
@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

wtf...

fatal: unable to auto-detect email address (got 'travis@travis-job-0df252a1-80af-4986-bcd2-c7b70463a4f2.(none)')

i set it in the environment and through git-config, so not sure what's going on:

  - git config --global user.name travis
  - git config --global user.email git-mediawiki@devnull.travis-ci.org
  - env GIT_AUTHOR_IDENT='travis <git-mediawiki@devnull.travis-ci.org>' GIT_COMMITTER_IDENT='travis <git-mediawiki@devnull.travis-ci.org>' GITPERLLIB="$TRAVIS_BUILD_DIR" make test

sigh.

@anarcat anarcat force-pushed the travis branch 4 times, most recently from bb63c64 to fd5ad58 Compare November 22, 2017 18:37
@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

/me pissing in the wind

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

holy crap i freaking made it. it's green.

i'll try to cleanup that horrible commitlog and push again, and then we're ready to fly Travis airlines. i'm sure it's going to go perfectly well.

This includes a webserver, PHP and Perl libraries necessary for both
MediaWiki and our git remote to work.
Add the sharness git repository as a submodule and use it throughout
instead of the Git test-lib.sh code.
Previous build system required the git build tools to be installed to
operate properly. Worse: the test suite required git to be *built* to
operate at all.

We now completely remove that dependency by doing our own "build" or
rather, by removing the need to do a build at all. We simply assume
/usr/bin/perl exists, and that we'll use the GITPERLLIB environment to
specify the path to our Git library during testing.

do not depend on an externally built git and assume it is available
@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

alright, that's a little better. now I need to open issues about the expected failures and we're done here.

@moy do you want to review this before I merge? or in general, should I just merge my own stuff or do you want eyes on them first?

For some reason, Travis doesn't have a hostname that git autodetects
correctly. Even worse, setting it with `git config --global` doesn't
seem to have effect in some tests. Take this job for example:

https://travis-ci.org/Git-Mediawiki/Git-Mediawiki/builds/305944550

The error there is:

    fatal: unable to auto-detect email address (got 'travis@travis-job-fe8c0839-f7af-4467-af53-d4af1d37af69.(none)')

Even though the email is explicitly set in the job itself. For some
reason, in some circumstances (but not all!), autodetection fails. I
suspect it has to do with git running in a deleted directory and
failing to find a local config, because the next job added debugging
information:

https://travis-ci.org/Git-Mediawiki/Git-Mediawiki/builds/305947191

And git config failed with this error:

    fatal: unable to read config file '/home/travis/build/Git-Mediawiki/Git-Mediawiki/t/trash directory.t9361-mw-to-git-push-pull.sh/.gitconfig': No such file or directory

In any case, there's an environment variable (`EMAIL`) used by git to
determine the user's email address that we can use here to fix those
failures.
@moy
Copy link
Collaborator

moy commented Nov 22, 2017

I had a quick look and saw nothing shocking. Unfortunately I don't have time for a better review.

On overall, do whatever you think is best for the project. Merge if you want, ask for feedback and wait if you prefer. It's probably a good idea to go through pull-requests even if you merge them right away for anything non-trivial. This way, watchers of the GitHub repo get a notification and see that the project is alive again (perhaps some potential contributors who were discouraged by the git.git submission process will step in now, who knows?).

In short: consider yourself the boss ;-).

We do not have that, nor is there a reason to use a custom perl
executable in our tests, so just use "perl" here.
This will allow us to test regressions at least. This may show that we
are now reproducing Git-Mediawiki#29, so we should keep an eye on those bugs.
@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

awesome, thank you for your confidence.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

i've rewritten history one last time, hopefully this will be easier to review than the horrible pile of commits i had earlier.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 22, 2017

i opened #56 regarding the test suite failures we ignored here. it also seems like one of those does not really fail reliably... anyways, i'll just merge this so we can check for regressions already.

@anarcat anarcat merged commit e408653 into Git-Mediawiki:master Nov 22, 2017
@anarcat anarcat deleted the travis branch November 23, 2017 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants