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

Code location hints for PHPT and @requires #3467

Conversation

@epdenouden
Copy link
Collaborator

@epdenouden epdenouden commented Jan 9, 2019

New functionality

Implemented code location hints for:

  • PHPT --SKIPIF--
  • PHPT --EXPECT--, --EXPECTF-- and --EXPECTREGEX--
  • PHPT external files referenced in --{SECTION}_EXTERNAL-- sections
  • @requires-annotations in testMethod docblocks

When a test gets marked skipped due to missing requirements PHPUnit will search through the relevant sections and annotations. For failing end-to-end tests it will try to locate the source of the problem in the $expected definition.

For unit tests the system uses metadata already available as a performance trade-off and can miss the exact line due to empty lines between docblocks and method definitions. This rarely happens as coding style fixers clean this scenario up.

For PHPT, the already parsed sections are searched for the exception message instead of trying to analyse the code in the sections. External files are reported to the user but not scanned for exact line numbers to keep it fast.

Misc improvements

  • first work on the basic test collection (#3453) is already in use to test --debug
  • added more @testdox descriptions
  • colorized Testdox
    • lower precision test duration to reduce punctuation clutter
    • visualize whitespace in failed assertion comparison output

Examples

Unit tests

./phpunit --testdox tests/_files/RequirementsTest.php

image

PHPT --SKIPIF--

./phpunit --testdox tests/end-to-end/_files/phpt-skipif-location-hint-example.phpt

image

Output in IntelliJ / PHPStorm with --teamcity option

image

@codecov
Copy link

@codecov codecov bot commented Jan 9, 2019

Codecov Report

Merging #3467 into master will increase coverage by 0.25%.
The diff coverage is 92.3%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3467      +/-   ##
============================================
+ Coverage     82.63%   82.88%   +0.25%     
- Complexity     3616     3636      +20     
============================================
  Files           144      144              
  Lines          9437     9513      +76     
============================================
+ Hits           7798     7885      +87     
+ Misses         1639     1628      -11
Impacted Files Coverage Δ Complexity Δ
src/Util/Color.php 100% <ø> (ø) 16 <0> (ø) ⬇️
src/Util/TestDox/CliTestDoxPrinter.php 95.49% <100%> (ø) 50 <0> (ø) ⬇️
src/Util/Test.php 93.05% <100%> (+0.28%) 207 <13> (ø) ⬇️
src/Runner/PhptTestCase.php 81.85% <82.97%> (+1.67%) 97 <12> (+14) ⬆️
src/Framework/TestCase.php 76.09% <92.85%> (+0.26%) 310 <6> (+6) ⬆️
src/TextUI/TestRunner.php 69.4% <0%> (+0.15%) 289% <0%> (ø) ⬇️
src/Util/Getopt.php 96.38% <0%> (+1.2%) 37% <0%> (ø) ⬇️
src/Util/Filter.php 95.12% <0%> (+7.31%) 21% <0%> (ø) ⬇️
src/Framework/SyntheticError.php 100% <0%> (+100%) 4% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d91c1c...11191a8. Read the comment docs.

Loading

@codecov
Copy link

@codecov codecov bot commented Jan 9, 2019

Codecov Report

Merging #3467 into master will increase coverage by 0.25%.
The diff coverage is 93.22%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3467      +/-   ##
============================================
+ Coverage     82.63%   82.88%   +0.25%     
- Complexity     3616     3638      +22     
============================================
  Files           144      144              
  Lines          9437     9517      +80     
============================================
+ Hits           7798     7888      +90     
+ Misses         1639     1629      -10
Impacted Files Coverage Δ Complexity Δ
src/Util/Color.php 100% <ø> (ø) 16 <0> (ø) ⬇️
src/Util/TestDox/CliTestDoxPrinter.php 95.49% <100%> (ø) 50 <0> (ø) ⬇️
src/Util/Test.php 93.05% <100%> (+0.28%) 207 <13> (ø) ⬇️
src/Framework/Assert.php 92.56% <100%> (+0.14%) 247 <6> (+7) ⬆️
src/Runner/PhptTestCase.php 81.85% <82.97%> (+1.67%) 97 <12> (+14) ⬆️
src/Util/XdebugFilterScriptGenerator.php 85.71% <0%> (-9.03%) 7% <0%> (+1%)
src/TextUI/TestRunner.php 69.4% <0%> (+0.15%) 289% <0%> (ø) ⬇️
src/Util/Getopt.php 96.38% <0%> (+1.2%) 37% <0%> (ø) ⬇️
src/Util/Filter.php 95.12% <0%> (+7.31%) 21% <0%> (ø) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d91c1c...3e910b3. Read the comment docs.

Loading

@epdenouden epdenouden changed the title WIP Code location hints for PHPT sections and @requires docblocks WIP Code location hints for PHPT sections and @requires docblocks Jan 9, 2019
@epdenouden epdenouden changed the title WIP Code location hints for PHPT sections and @requires docblocks WIP Code location hints for PHPT and @requires Jan 9, 2019
@epdenouden epdenouden changed the title WIP Code location hints for PHPT and @requires Code location hints for PHPT and @requires Jan 10, 2019
@epdenouden
Copy link
Collaborator Author

@epdenouden epdenouden commented Jan 10, 2019

@sebastianbergmann While experimenting with test collections I ended up with a bunch of regexes, shell scripts and too many tabs open. This pull request is the result of consolidating the scripts into a clean way to add new code location hints to the verbose stack traces.

The whitespace visualization together with new code hints is a really nice help when debugging end-to-end tests, like the loggers. I will refine it further in future pull requests for the Basic test suite. The new test duration format makes the colored testdox quieter and easier to read; no more lists of random 0.* ms. 🤐

Have a look and let me know if you see anything that needs work.

Loading

@epdenouden epdenouden changed the title Code location hints for PHPT and @requires WIP Code location hints for PHPT and @requires Jan 10, 2019
@sebastianbergmann sebastianbergmann merged commit 5189727 into sebastianbergmann:master Jan 11, 2019
4 checks passed
Loading
@epdenouden epdenouden deleted the issue-3453-basic-test-collection branch Jan 11, 2019
@epdenouden
Copy link
Collaborator Author

@epdenouden epdenouden commented Jan 11, 2019

@sebastianbergmann Ah well, I found a missing case. For some reason $e->getComparisonFailure()
returns NULL in some cases, causing ->getDiff() to fail eventhough the exception message contains diff output.

I'll have working code hints for that scenario via a workaround soon, but any insight into what's going on would be great. From PhptTestCase::run()

if ($e instanceof ExpectationFailedException) {
    /** @var ExpectationFailedException $e */
    if ($e->getComparisonFailure()) {
        $diff = $e->getComparisonFailure()->getDiff();
    } else {
        $diff = $e->getMessage();
    }

    $hint = $this->getLocationHintFromDiff($diff, $sections);
    $trace = \debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS);
    \array_unshift($trace, $hint);
    $failure = new PHPTAssertionFailedError($e->getMessage(), 0, $trace[0]['file'], $trace[0]['line'], $trace);
//  \print_r(['hint' => $hint]);
}

Loading

@epdenouden epdenouden changed the title WIP Code location hints for PHPT and @requires Code location hints for PHPT and @requires Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants