Skip to content

Improve parsing of test files to locate functions #52

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

Merged
merged 11 commits into from
Jan 15, 2018

Conversation

reubent
Copy link
Contributor

@reubent reubent commented Jan 3, 2018

No description provided.

@reubent
Copy link
Contributor Author

reubent commented Jan 3, 2018

I didn't actually mean to submit this - force of habit creating pull requests after pushing kicked in. This was written for our own internal use but feel free to use if it is helpful. It works around a number of issues we were having with parsing of test files.

@thibaultfalque
Copy link
Collaborator

@reubent Thanks you for your contribution !

@danielleberre
Copy link
Contributor

Thanks @reubent for your contrib. It will probably fix some issues for other users of the plugin too (e.g. #49).

Would it be possible to make your PR against the maintenance branch instead of master?

The maintenance branch contains the 1.2.X releases code while the master branch contains code for the next major release.

It would be nice too if you could prevent pom.xml to be changed in the PR :).

@Tvli
Copy link
Contributor

Tvli commented Jan 5, 2018

Hi @reubent @danielleberre
It looks like this PR breaks the bug I fixed in #27 .
Please take a look in the above link for the detailed explanation.
Also please make sure the new tests covers all the existing tests. As far as I can tell the tests for people who use testSuite have been removed.

@danielleberre
Copy link
Contributor

Thanks @Tvli for pointing that out.
I guess that we could add the following test sample to test_test.go to make sure that the proposed REGEXP also works for you?

func (suite *ExampleTestSuite) TestExample() {
    suite.Equal(5, suite.VariableThatShouldStartAtFive)
}

@danielleberre danielleberre changed the base branch from master to maintenance January 5, 2018 17:46
@danielleberre
Copy link
Contributor

I resolved the conflicts in reubent-fix_test_parsing branch.

Please check that everything is fine (we may want to create a new PR from that branch).

@Tvli your test case has been added in the file test_test.go, and the test case now breaks. So we can improve the PR with your specific testcase.

@thibaultfalque the build is successful while there are failing test cases, as such the CI is not very useful right now for PR ...

thibaultfalque added a commit that referenced this pull request Jan 5, 2018
thibaultfalque added a commit that referenced this pull request Jan 5, 2018
thibaultfalque added a commit that referenced this pull request Jan 5, 2018
@Tvli
Copy link
Contributor

Tvli commented Jan 5, 2018

Hey @danielleberre

The test case looks good to me. Thanks 👍

@danielleberre
Copy link
Contributor

@Tvli I updated the REGEXP in a282eee to match your specific needs. I assume that testsuite always start with (suite, is that correct?

thibaultfalque pushed a commit that referenced this pull request Jan 6, 2018
thibaultfalque pushed a commit that referenced this pull request Jan 6, 2018
@Tvli
Copy link
Contributor

Tvli commented Jan 6, 2018

Hi @danielleberre
No it's not. In Go lang , the syntax for a struct instance method is

func (alias struct) methodName() {}

To be more exact:

type A struct {
  Val string 
}

func (whateverIWant A) printVal() {
  print(whateverIwant.Val)
}

so the "suite" can be any alias people want to use to represent the struct within the function's scope.

I guess you probably want to find the Test function name after the first ).

danielleberre added a commit that referenced this pull request Jan 7, 2018
@danielleberre
Copy link
Contributor

@Tvli Thanks for the explanation. I added a new test case and fixed now properly the REGEXP.

@thibaultfalque thibaultfalque merged commit 5d75ade into uartois:maintenance Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants