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

Migrate to PHPUnit 6.0 (related to PHP runtime) #117

Closed
3 tasks done
ghost opened this issue Feb 19, 2017 · 12 comments
Closed
3 tasks done

Migrate to PHPUnit 6.0 (related to PHP runtime) #117

ghost opened this issue Feb 19, 2017 · 12 comments
Milestone

Comments

@ghost
Copy link

ghost commented Feb 19, 2017

Now PHPUnit 6.x is stable and previous versions (< 6.0) are outdated. The main change is changing the naming of classes and introducing namespaces: the old class names in format PHPUnit_Framework_TestCase (no namespaces) was replaced with \PHPUnit\Framework\TestCase (i.e. class TestCase under the PHPUnit\Framework namespace).

Here are steps to migrate to PHPUnit 6.0:

  • Fix .travis.yml - it seems like it uses local outdated phpunit executable and not downloaded one, here is an excerpt from CI logs:
$ wget https://phar.phpunit.de/phpunit.phar
# ...
# PHPUnit 6.0.6 is downloaded
Location: https://phar.phpunit.de/phpunit-6.0.6.phar [following]
...
# but the next command displays 5.x.y version:
$ phpunit --version
PHPUnit 5.3.4 by Sebastian Bergmann and contributors.

To fix it we could use absolute paths or remove the existing already installed phpunit, something like that:

  # PHP: newer PHPUnit, as the one in the repos is very old
  - phpenv global 7.0.7
  - php --version
  - "sudo rm -f $(command -v phpunit) && sudo curl -oL /usr/local/bin/phpunit https://phar.phpunit.de/phpunit.phar && sudo chmod +x /usr/local/bin/phpunit"
  # Should show `PHPUnit 6.x.y by Sebastian Bergmann and contributors.`
  - phpunit --version
<?php
namespace Kaitai\Struct\Tests;

abstract class TestCase extends \PHPUnit\Framework\TestCase {
    const SRC_DIR_PATH = __DIR__ . '/../../src';
}

@GreyCat I will update the kaitai_struct_php_runtime to use PHPUnit 6.x.

@GreyCat
Copy link
Member

GreyCat commented Feb 19, 2017

Would having language: php (and thus using PHP-specific Travis VM) help in this case? If yes, then may be we can postpone it until #62?

@ghost
Copy link
Author

ghost commented Feb 19, 2017

I don't have info about what version of PHPUnit they have installed for the language: php.

@GreyCat GreyCat changed the title Mirgrate to PHPUnit 6.0 (related to PHP runtime) Migrate to PHPUnit 6.0 (related to PHP runtime) Feb 20, 2017
@ghost
Copy link
Author

ghost commented Feb 25, 2017

@GreyCat GreyCat added this to the v0.7 milestone Feb 25, 2017
GreyCat added a commit to kaitai-io/kaitai_struct_tests that referenced this issue Feb 28, 2017
@GreyCat
Copy link
Member

GreyCat commented Feb 28, 2017

I've updated .travis.yml and TestCase.php, now it seems that PHPUnit 6 is working: http://kaitai.io/ci/

However, a few tests seems to be broken with "Risky test" diagnosis. Can you take a look?

@ghost
Copy link
Author

ghost commented Feb 28, 2017

OK

@ghost
Copy link
Author

ghost commented Feb 28, 2017

Risky tests are tests without assertions, i.e. without any $this->assert*() method, they can be considered as notification for developers. If test should not do any assertions, the easiest way to fix such test is adding $this->assertTrue(true);, e.g.:

class DocstringsTest extends TestCase {
    public function testDocstrings() {
        $r = Docstrings::fromFile(self::SRC_DIR_PATH . "/fixed_struct.bin");
        // This test should not do any assertions, so use $this->assertTrue(true) to mark it as not Risky (R).
        $this->assertTrue(true); 
    }
}

We could move the $this->assertTrue(true) into the Kaitai\Struct\Tests\TestCase to avoid adding comments as above:

    protected function markTestAsNotRisky() {
        // Another way to do this: $this->addToAssertionCount(1);
        $this->assertTrue(true) may work too.
    }

There are 2 useful methods also:

// Mark test incomplete, it is useful for not completed yet tests, which should be completed later.
$this->markTestIncomplete();

// Mark test as skipped, it is useful for tests, which should be ignored and marked as Skipped (S).
// E.g. tests which should work only under Linux and ignored under Windows.
$this->markTestSkipped();

E.g. of usage:

class DebugEnumNameTest extends TestCase {
    public function testDebugEnumName() {
        $r = DebugEnumName::fromFile(self::SRC_DIR_PATH . "/fixed_struct.bin");

        # this test is meaningful only for languages that have --debug and do
        # not save enum type info
        $this->markTestSkipped();
    }
}

@GreyCat
Copy link
Member

GreyCat commented Feb 28, 2017

Thanks for the explanation! Could you add these fixes then? I'll analyze and add support for skipped tests as "green" ones.

@ghost
Copy link
Author

ghost commented Mar 1, 2017

Ok, the pull request has been created.

@ghost
Copy link
Author

ghost commented Mar 1, 2017

Is that ok for you if we close this issue?

@GreyCat
Copy link
Member

GreyCat commented Mar 1, 2017

Just a little more, I'd like to ensure that "risky" and "skipped" are now rendered properly ;)

@GreyCat
Copy link
Member

GreyCat commented Mar 19, 2017

Ok, better late than never ;) Sorry for the delays. Got it working, result can be seen on CI. PHP should be back to 100% now.

@ghost
Copy link
Author

ghost commented Mar 19, 2017

Great, thank you!

@ghost ghost closed this as completed Mar 19, 2017
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant