[DO NOT MERGE] Safe mode (array parser changes only)

battye commented 5 years ago

EDIT: this PR only exists for reviewing the safe mode code changes by themselves

Usage

To run (using the ja language for example):

 php src/Phpbb/TranslationValidator/PhpbbTranslationValidator.php validate ja --phpbb-version=3.2 --safe-mode --display-notices

Changes

Web safe mode
- Uses the flag --safe-mode
- The idea being we can integrate this into Titania and run on a website; so we have to parse language files instead of including them where code could be executed
- I've included notices in places where we can't get the same coverage from parsing as from including
Removed 3.0/3.1 references (#42)
Unit tests are fixed now (they hadn't been passing since https://travis-ci.org/phpbb/phpbb-translation-validator/jobs/229722982)

PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

Runtime:       PHP 5.6.15-1+deb.sury.org~trusty+1 with Xdebug 2.3.2
Configuration: /home/vagrant/phpbb/phpBB/array_parser/phpbb-translation-validator/phpunit.xml

......................R........................................  63 / 108 ( 58%)
.............................................                   108 / 108 (100%)

Time: 4.87 seconds, Memory: 11.00MB

There was 1 risky test:

1) Phpbb\TranslationValidator\Tests\FileValidator\ValidateLangTest::testValidateLangFile with data set #2 ('language/lang2.php', array('4-Must only contain the lang-...2.php-'))
Test code or tested code did not (only) close its own output buffers

OK, but incomplete, skipped, or risky tests!
Tests: 108, Assertions: 108, Risky: 1.

Process finished with exit code 0

Outstanding tasks

It would be good to test against a whole bunch of languages, to give the array parser a workout
Regression testing now that 3.0/3.1 support is removed, none of the 3.2 functionality is broken
@Crizz0 noticed an issue with class paths depending on where the script was executed from, are you able to explain again how you were able to get that error and what you did to fix it?

battye commented 5 years ago

I haven't merged this into the PR yet, because I wanted to double check something @Crizz0 - is there a particular reason why we are using the .phar / .bat file approach?

It just seems to be adding an extra layer of complexity I think. I've refactored the script now so that you can just do a git clone and composer install, then run:

php translation.php validate fr

And if you don't want to put a 3.2 directory inside root, you can specify it explicitly like:

php translation.php validate fr --package-dir=/path/to/where/is/3.2

So pending your response, I've just kept this as a separate branch at: https://github.com/battye/phpbb-translation-validator/pull/1 but I think it makes more sense to do it this way. I also moved all of the unit tests out of the src directory and put them into the tests directory.

Crizz0 commented 5 years ago

The validator should work on MacOS, Linux and Windows. Maybe that is why the .bat/.phar version was chosen.

Maybe we should create a "develop"-branch in this repo to make PR against "develop", create feature-branch on it and and only merge working develop-branches into "master".

battye commented 5 years ago

That's a good idea, are you able to create a develop branch on here (I don't have permissions) and I'll create a PR for it?

Crizz0 commented 5 years ago

I think so.

battye commented 5 years ago

Thanks, I've added a new PR #54 So is the idea that we'll merge that in now and use the develop branch here for testing?

battye commented 5 years ago

Don't need this PR anymore. Everything is in https://github.com/phpbb/phpbb-translation-validator/pull/54 now.

phpbb / phpbb-translation-validator