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

Phpunit testing #4994

Conversation

bartlomiej-laczkowski
Copy link

What does this PR do?

This pull request adds support for PHPUnit testing in Che.

What issues does this PR fix or reference?

#3987

Changelog

Support for PHPUnit testing in Che.

Release Notes

N/A yet

Docs PR

N/A yet

Bartlomiej Laczkowski added 13 commits February 21, 2017 21:52
Signed-off-by: Bartlomiej Laczkowski <[email protected]>
Signed-off-by: Bartlomiej Laczkowski <[email protected]>
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@bartlomiej-laczkowski
Copy link
Author

Just to clarify some things, this pull request in current state provides also some general changes in testing model both on the server and IDE side. While working on this PR I have noticed that in the meantime some changes were added, which affects testing framework in general (i.e. PR #4481). I was able to merge those changes into this PR, anyway the changes that I have made also affect testing framework in general and I think that someone from Codenvy and @davidfestal who added some recent changes in Java testing should take a look at this patch and I would be grateful for any discussion about possibility of "merging" the changes that come with this PR.

Below is the list of most important changes in testing framework that are a part of this PR:

  • New DTO's for test result and related data (see new classes in org.eclipse.che.api.testing.shared.dto package)
  • New data nodes for creating result tree in Test Result view (see new classes in org.eclipse.che.plugin.testing.ide.view.navigation.nodes package)
  • Enhanced panels for presenting test result tree and related failures stack traces (new types of possible test result like warning, skipped test, etc.)

All of the classes and methods in testing framework internals that in my opinion should be changed/replaced with the ones provided in this PR were marked with @deprecated annotation.

I am eager to answer all the questions related to the changes and ready to make necessary tweaks in this PR if the proposed changes are too intrusive.

@davidfestal
Copy link
Contributor

@bartlomiej-laczkowski I'll have a look today, or asap. BTW, this issue also seems related to the work done on branch https://github.com/eclipse/che/tree/new-java-test for issue #4679. @evidolob wdyt ?

@bartlomiej-laczkowski
Copy link
Author

@davidfestal and whoever who might be related to Che unit testing: any feedback?

@davidfestal
Copy link
Contributor

@bartlomiej-laczkowski: regarding your action group refactoring, you should be aware of PR #4980 that somehow merges the project-explorer contextual menus and top-bar menus, to provide a consistent behaviour throughout the IDE.

As for the API changes you introduced in the TestRunner with new DTOs, I would be interested in understanding their overall purpose (maybe I would get an idea by deeply reading all the changes, but there are many...).

@benoitf benoitf requested a review from vparfonov May 11, 2017 08:05
@benoitf benoitf added kind/enhancement A feature request - must adhere to the feature request template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels May 11, 2017
@slemeur
Copy link
Contributor

slemeur commented May 30, 2017

Hi @bartlomiej-laczkowski. Thanks for your PR.

We need @kaloyan-raev assistance in order to review the PR. His expertise on PHP will be helpful here. We'll have @vparfonov also reviewing the code.

Separately, @bartlomiej-laczkowski we need you to document a little bit more your proposed features:

  • Documentation for end-user will be needed
  • Sample use cases so that we can author selenium tests

Thanks in advance!

@slemeur slemeur requested a review from kaloyan-raev May 30, 2017 15:49
@kaloyan-raev
Copy link
Contributor

@slemeur I will build this branch and check if the PHPUnit implementation works as expected. I also made @bartlomiej-laczkowski aware of the https://github.com/eclipse/che/wiki/Coding-Guidelines

Signed-off-by: Bartlomiej Laczkowski <[email protected]>
Copy link
Contributor

@kaloyan-raev kaloyan-raev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built and tested the PR. Everything works as expected from functional point of view. Tested with both PHP and Zend stacks.

In the PHP stack you can follow the PHP tutorial from the docs and run the test.php with the 'Run Test > PHPUnit' action from the context menu.

In the Zend stack you can use the 'zend-expressive' project template and then use 'Run Test > PHPUnit' on project level, or on the test folder, or on a specific test file under the test folder.

The test results appear in the 'Test Results' view and are correctly marked as successful or failed. Double-clicking on a test opens it in the PHP editor. Failed tests have failure details and stack trace displayed on the right side of the view. Double-clicking on a stack trace element opens it in the PHP editor.

@kaloyan-raev
Copy link
Contributor

I prepared PR eclipse-che/che-docs#241 to update the PHP tutorial with the new PHPUnit plugin.

However, I don't find any general doc page about testing in Che Docs like we have for debugging. Is there a place where JUnit and TestNG are documented? If there is, I can update it with PHPUnit too.

@bartlomiej-laczkowski
Copy link
Author

@slemeur @vparfonov

Just before creating any doc etc. and to help the reviewer to do his best, below is the short description of most important changes & new parts of code. As I already mentioned in one of the previous comments those changes are new part of API that are the "second door" for providing test results to IDE part of Che. Some part of already existing interfaces, DTO's and methods were marked as deprecated just to help migrating JUnit and TestNG support to new part of API. I have decided to provide new parts of API and change some existing ones as it provides new possibilities that were just simply not available in previous state of testing support API and it was very hard to change anything as there were some ongoing changes in unit testing framework in the meantime.

IDE part:

  • TestServiceClient :
    • runTests(String,String, Map<String, String>) - alternative/replacement for run() method which returns new TestResultRootDto.
    • getTestResults(String, List<String>) - new wethod for fetching test result child elements (test suites & test cases). It gives possibility to lazily load child elements in unit testing result tree (similar approach as it is for fetching child variables in debug views).
  • TestActionRunner - new common class for running test actions, for now it overlaps with RunTestActionDelegate but I am not sure if it would be a good replacement.
  • TestResultPresenter :
    • handleResponse(TestResultRootDto) - alternative/replacement for handleResponse(TestResult) that handles TestResultRootDto as input
  • TestResultViewImpl :
    • Some improvements in UI, especially new way of presenting trace frames (right output panel) with the use of error message above and list of "clickable" trace frames below (user may open related source files if there are any), lazy loading of test result tree children, additional info label (for PHPUnit it shows tests execution time)
    • showResults(TestResultRootDto) - alternative/replacement for showResults(TestResult) that presents test results based on TestResultRootDto
    • clear() - new method for clearing result output views whenever new test are being run.
  • SimpleLocationHandler - new class for opening test case or trace frame related source file (see also SimpleLocationDto)
  • TestResultNodeFactory
    • Added new methods for creating new types of nodes
  • New nodes for presentation:
    • TestResultRootNode - for general/root result
    • TestResultNode - for test suite or test case
    • TestResultTraceFrameNode - for test case related trace frame

SHARED part:

  • org.eclipse.che.api.testing.shared.common package:
    • TestResultStatus - enum for diferent kinds of possible test result statuses (i.e. error, success, warning, failure, skipped - all those kinds are supported by PHPUnit)
    • TestResultType - enum for indicating test result type (test suite or test case)
  • org.eclipse.che.api.testing.shared.dto package:
    • TestResultRootDto - DTO for root result info, holds info about child result DTO 's
    • TestResultDto - DTO for test suite/test case info, holds info about trace DTO
    • TestResultTraceDto - DTO for storing test result error message and child trace frames
    • TestResultTraceFrameDto - DTO for single trace frame, stores info about frame description and related source file location
    • SimpleLocationDto - DTO for storing info about source file locations

SERVER part:

  • TestingService & TestRunner :
    • runTests(String, String, Map<String, String>) and getTestResults(String, List<String>) - the same goals as for TestServiceClient in IDE part
  • TestFrameworkException - new exception type for handling info about exceptions while performing tests

@slemeur
Copy link
Contributor

slemeur commented Jun 9, 2017

Thanks a lot for those clear explanation @bartlomiej-laczkowski ! This is excellent.

@vparfonov : Do you need anything more to complete the review and have QA adding tests?

@vparfonov
Copy link
Contributor

ci-build

@codenvy-ci
Copy link

Build # 2884 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2884/ to view the results.

@vparfonov
Copy link
Contributor

@bartlomiej-laczkowski we are ready to merge it now but it is out of date. Can you please update it according to the master branch? Thanks

@bartlomiej-laczkowski
Copy link
Author

@vparfonov I have updated the PR with recent master.

@vparfonov
Copy link
Contributor

@bartlomiej-laczkowski thanks

@vparfonov
Copy link
Contributor

merged here #5468 thakns for your work

@vparfonov vparfonov closed this Jun 26, 2017
@bartlomiej-laczkowski
Copy link
Author

@vparfonov You are welcome and thanks!

@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template. sprint/current
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants