Test architecture implementation

eugene-augier commented 2 years ago

Hi :wave:

I will use the pronoun we even though I'm not a long-time contributor because this is the pronoun that is usually appropriate to opensource :wink:.

Following this discussion: https://github.com/php/doc-en/pull/1536 I understand that PHP GROUP cannot use third-party libraries so as not to support them.

Problems

This forces us to develop our own tools, which can be time consuming, have we got the resources?
Developing and using components can add extremely sneaky bugs that can impact the application in many places...

But, I think PHP deserves this time and I have a lot of free time at the moment so I would like to contribute to an opensource project. I have basic knowledge in C and interpretation of programming languages, but I prefer to start here if you think I can be useful.

In short, if you allow me to work on it, I am that resource :+1:

Solution

I developed a testing component, it's a fairly simple component that allows you to create complete test classes with assertions. There are no complex features such as mock but I think basic assertions are more than enough.

Btw, the component is tested by itself.

Direct improvements:

It allows to centralize test processing, greatly benefit from dependency injection and autoloading.
We can start to develop highly subject to errors tools like Lexer/Linter without (or much more less) being afraid of causing vicious bugs.

You can already see the interest of developing components because some have been created for the library (Output styling, File Resolver, String Dumper...).

Future improvements?

These components could have their own repository and be used in other PHP GROUP projects?
More centralization and possibilities thanks to an executable bin/console?

Examples:

php bin/console linter --fix /my-folder

php bin/console generate:page-doc

Break something?

This PR does not modify any existing code so the evolution can be progressive and experimental as far as I can tell.

How it works?

Launch all tests:

php bin/test

Launch tests under a specified folder:

php bin/test components/String

Testing a specific testMethod in a specific class:

php bin/test components/String/__test__/DumperTest.php testDump

According to the current configuration (so it may change):

test classes must be placed under a __test__ folder, must be suffixed with Test and implements PHPDoc\Internal\Testing\AssertInterface or extends PHPDoc\Internal\Testing\Assert (which implements AssertInterface).
Test methods must be prefixed with test, e.g. testDump.
test methods that should be run but cannot be run at the moment should be prefixed with skip, e.g. skipDump.

Example:

use PHPDoc\Internal\String\Dumper;
use PHPDoc\Internal\Testing\Assert;

class DumperTest extends Assert
{
    public function testDump()
    {
        $this->assertSame('"not-foo"', Dumper::dump('foo')); // fail
        $this->assertSame('true', Dumper::dump(true)); // pass

        // code..
    }

    public function skipTest()
    {
        $this->assertFalse(true); // skipped so doesn't perform any assertion
    }
}

Lauch the test:

php bin/test components/String/__test__/DumperTest.php

Output:

Output after fix:

Can print complex data structure thanks to Dumper:

@Girgias gave me some feedbacks, I will let him express himself his views on the PR

I am available for your feedback, corrections, improvements of course :)

eugene-augier commented 2 years ago

I had forgotten to commit the test binary...

Girgias commented 2 years ago

Before going into my opinion on the PR as a whole:

The files are going to need license header comments AFAIK. I would have expected the binary test file be marked as executable. And I'm hitting some assertion issues on Fedora 35:

[girgias@localhost base]$ php bin/test 
test: testDeepLoad
file: /home/girgias/Dev/php-docs/base/components/Loading/__test__/FileResolveTest.php
line: 69
given: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/b.yml"
should be: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/bar/a.txt"
test: testDeepLoad
file: /home/girgias/Dev/php-docs/base/components/Loading/__test__/FileResolveTest.php
line: 70
given: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/bar/a.txt"
should be: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/b.yml"
test: testDeepExclude
file: /home/girgias/Dev/php-docs/base/components/Loading/__test__/FileResolveTest.php
line: 109
given: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/a.php"
should be: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/a.txt"
test: testDeepExclude
file: /home/girgias/Dev/php-docs/base/components/Loading/__test__/FileResolveTest.php
line: 110
given: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/b.yml"
should be: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/a.php"
test: testDeepExclude
file: /home/girgias/Dev/php-docs/base/components/Loading/__test__/FileResolveTest.php
line: 111
given: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/a.txt"
should be: "/home/girgias/Dev/php-docs/base/components/Loading/__test__/fixtures/foo/b.yml"
test: testFileWithoutClass
file: /home/girgias/Dev/php-docs/base/components/Testing/__test__/TesterTest.php
line: 44
given: "Class "DoesNotImplementsAssertInterfaceTest" must implements "PHPDoc\Internal\Testing\AssertInterface" interface or extends "PHPDoc\Internal\Testing\Assert""
should be: "Class "NotAClassTest" does not exists"

6 assertion(s) failed...

I personally think it can make sense to have our own test runner for the docs. One question then is "why not use run-test from php-src?" as this is already a custom test runner with a file format. In any case, this is going to need to have a file like a README explaining how to write tests. But maybe OTH it is better to bite the bullet and use an already established userland library for testing purposes.

In any case, this on its own does not provide much value, but it would enable us to write more resilient tools instead of manually checking if the scripts work. The main reason I think this might benefit from being it's own repo is that we could use it with PhD. As currently it uses the php-src test runner, but we can't easily include it in CI. Moreover, most of the tests are currently broken and making changes to it is pretty risky and prone to error.

Just as a final note, I don't really understand how the FileResolver class is meant to work/be used.

eugene-augier commented 2 years ago

Related to errors, it seems that files are not loaded in the same order for you. I will look for a testing alternative.

One question then is "why not use run-test from php-src?"

I don't know how this component works so you probably know better than me if it's a good idea or not to use it. My first thought is that my test library is designed specifically for testing application layer elements and not php itself.

In any case, this is going to need to have a file like a README explaining how to write tests.

Yes, I will redact it when you will be okay with the code.

Just as a final note, I don't really understand how the FileResolver class is meant to work/be used.

It's just to retrieve file easier, the tests related to it show the goal and the way to use it.

In any case, this on its own does not provide much value, but it would enable us to write more resilient tools instead of manually checking if the scripts work.

Yes I am afraid that having a good testing tool is a prerequisites to improve the global quality of qa and doc-base itself. My library or another one but I can't judge what is the best or is possible for this question.

eugene-augier commented 2 years ago

Some examples of lexer/linter development tested by the tool are available here: https://github.com/eugene-augier/doc-base/pull/1/files#diff-fa4cc2a3aca904e54164afd4b54bfaa4b1e043c9b71e423789d73144612e6b61

php / doc-base