| Author |
 Topic  |
|
|
legalize
Tomato Guru
USA
119 Posts |
 Posted - May 12 2009 : 1:21:33 PM
|
| I have a large set of tests that I created for "that other refactoring add-on for C++". The tests are situated around their refactoring support, but I imagine that Whole Tomato could use the same suite of tests for VAX to make sure they cover all the sticky C++ cases correctly. How can I get the test suite (a ZIP of commented source), with several hundred test cases in it, to Whole Tomato? |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on May 12 2009 1:21:45 PM |
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - May 12 2009 : 2:56:01 PM
|
Please submit the files via the form:
http://www.wholetomato.com/support/contact.asp
including this thread ID or URL in the description, so we can match it up.
I already have a test suite that I use, but I am very interested in having a look at yours. Working out and then writing the test code can take a while, as I am sure you realise  |
zen is the art of being at one with the two'ness |
 |
|
|
legalize
Tomato Guru
USA
119 Posts |
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - May 20 2009 : 11:59:38 AM
|
| I have the files, many thanks for these. It is going to take me a while to read through all of this and compare it with my current test suite. It is going to make very interesting reading though |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - May 20 2009 : 12:18:19 PM
|
| Definately start out with rename and extract method/extract function. I have noticed cases where VAX doesn't get all the names in Rename and cases where the function/method is improperly extracted with Extract Function/Method. I haven't run this test suite against VAX yet, I've been too busy. |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - May 20 2009 : 6:35:27 PM
|
| I will start there, thank you. Both Rename and Extract Method should be very reliable over all, but there are a few known edge cases that cause problems. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - May 22 2009 : 12:29:24 AM
|
| Extract Method seems to most often have problems inferring the return type of the extracted method. Its flow analysis seems a little weak. There are probably cases in that suite that will bear this out. |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - May 31 2009 : 4:59:58 PM
|
I have started looking at this, and the second Extract Method test fails. This is the test:
void ConstOperation() const
{
// #TEST#: EM19 Extract Method on next line
int x = Function1() + Function2();
int y = x*2;
}
This is the expected result, and is somewhere between a bug and a feature. Currently VA expects you to have selected all uses of a local variable if you have selected its declaration. We are aware of this, and at the very least need to warn the user there is a possible problem when doing the extract:
case=2063
This probably explains most of the problems you have experienced with Extract Method. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - Jun 01 2009 : 12:00:59 PM
|
It depends on the details of the code. This is a simple example from my tests of Extract Method:
static int checkParametersArePickedUp(int x, int y, int z)
{
// test - select the marked code and trigger extract method
return x * y * z;
}
here the extracted function returns the result of "x * y", which is basically what you are asking about. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Jun 01 2009 : 1:36:28 PM
|
| But that's not the code we're discussing in the test case. Since you say it depends on the code in question, what happens when you select the right hand side of the assignment in that test case? |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - Jun 01 2009 : 1:51:40 PM
|
I think we are talking about different things here. Ignore the assignment for the moment. If you select the declaration of a local variable, but do not select all lines where the local variable is used then Extract Method with do the "wrong" thing. This is a known limitation.
It might break an assignment line, or something else. The problem is due to how local variables are handled, not how assignment is handled.
There might also be problems with how assignment is handled, but that would be a separate problem. There are various edge case problems in VA. We are working on them one by one |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Feb 05 2010 : 6:14:18 PM
|
I recently released the test suite here: http://legalizeadulthood.wordpress.com/2010/02/02/c-refactoring-tools-test-suite-available/
I'm running VAX against the tests, so I will have a bunch of bugs to report. Do you have a bug tracker site, or do I just post a forum thread?
For instance, here are the results for Rename:
= Rename
R1 Failure (doesn't select all instances of identifier)
R2 Failure (doesn't select all instances of identifier)
R3 Failure (doesn't select all instances of identifier)
R4 Failure (doesn't select all instances of identifier)
R5 Pass
R6 Pass
R7 Pass
R8 Pass
R9 Pass
R10 Pass
R11 Pass
R12 Pass
R13 Pass
R14 Failure (doesn't select all instances of identifier)
R15 Pass
R16 Pass
R17 Pass
R18 Pass
R19 Failure (doesn't select all instances of identifier)
R20 Failure (doesn't select all instances of identifier)
R21 Pass
R22 Pass
R23 Pass
R24 Pass
R25 Failure (not available)
R26 Pass
R27 Failure (doesn't select all instances of identifier)
R28 Pass
R29 Failure (doesn't select all instances of identifier)
R30 Pass
R31 Pass
R32 Failure (suggested identifier contains ~ and is allowed)
R33 Failure (doesn't select all instances of identifier)
R34 Failure (doesn't select all instances of identifier)
R35 Failure (not available)
R36 Failure (doesn't select all instances of identifier)
R37 Pass
R38 Pass
R39 Pass
R40 Pass
R41 Failure (not available)
R42 Pass
R43 Pass
R44 Pass
R45 Failure (not available)
R46 Pass
R47 Pass
R48 Pass
R49 Pass
R50 Pass
R51 Pass
R52 Pass
R53 Pass
R54 Pass
R55 Pass
R56 Pass
R57 Pass
R58 Pass
R59 Pass
R60 Pass
R61 Pass
R62 Pass
R63 Pass
R64 Pass
R65 Pass
R66 Pass
R67 Pass
R68 Pass
R69 Pass
R70 Pass
R71 Failure (doesn't select all isntances of identifier)
R72 Failure (doesn't select all isntances of identifier)
R73 Failure (doesn't select all isntances of identifier)
R74 Pass
R75 Pass
R76 Pass
R77 Failure (not available)
R78 Pass
R79 Pass
R80 Pass
R81 Failure (not available)
R82 Failure (doesn't select all instances of identifier)
R83 Failure (doesn't select all instances of identifier)
R84 Pass
R85 Pass
R86 Pass
R87 Pass
R88 Pass
R89 Pass
R90 Pass
R91 Pass
R92 Pass
R93 Pass
R94 Pass
R95 Pass
R96 Pass
R97 Pass
R98 Pass
75.52% Rename: 74 passed, 24 failed
0 Passed all tests:
Total: 75.52% (74 passed + 24 failed = 98 total)
|
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - Feb 08 2010 : 09:37:17 AM
|
| Downloading the test suite now, thank you for this. It is probably going to take me a little while to work through all of these tests. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - Feb 10 2010 : 2:22:23 PM
|
I need to compare all of the failed tests with our bug tracking system, since adding duplicate bug reports is unhelpful. I also like to understand the problem before putting in a bug report, to try and make sure that I am putting in the right bug report.
Ideally I also want to compare all of the tests that pass to the set of automated Rename tests that are run on new builds of VA, and add any tests that are not part of the current set. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Feb 10 2010 : 6:15:21 PM
|
| There seem to be three or four categories of problem picked up by the test cases. When you rename an identifier, sometimes incorrect identifiers are included in the selection list. For instance, two classes with the same name local to different source files. Neither can see the other, but rename on one picks up some instances of the other identifier from the other file. Sometimes it gets the right identifier but doesn't include all instances of that identifier. Renaming goto labels has this problem. Case R32 renames a destructor and the ~ from the method name is shown in the identifier box. Cases where the rename isn't available work properly for other occurrences of the same identifier, but don't recognize the identifier in all cases as something to be renamed. It doesn't appear that each one of these test cases pinpoints its own unique bug, but rather that the same bug manifests itself in several of the test cases failing. |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - Feb 11 2010 : 3:16:20 PM
|
This is going to take a little longer than expected. Just looking at Rename test 1, I am seeing a different, and very odd result. I am not sure that any instances of the class name are being missed, but VA does seem to be picking up to many instances.
But the code is not immediately clear to me, due to the fact that the symbol name "Rename1" has been used several times, in different contexts and scopes, so it will take time to check and understand every reference to the symbol in the code. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Feb 11 2010 : 3:44:36 PM
|
// #TEST#: R1 Rename this class, but class in Rename2.cpp shouldn't be renamed
This is the scenario where you have two local classes in different .cpp files (or one local and one published in a .h) that have the same name. Ideally, renaming one should not rename the other. If, however, you're going to present choices for the class in Rename2.cpp, then you should provide all the chocies.
If I accept the default selection and rename this class to Rename1zz and compile the solution, clearly instances of the identifier were missed:
1>------ Build started: Project: RefactorTest, Configuration: Debug Win32 ------
1>Compiling...
1>Rename.cpp
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(59) : error C2590: 'Rename1zz' : only a constructor can have a base/member initializer list
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(249) : error C2653: 'Rename1' : is not a class or namespace name
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(249) : error C2065: 'MyClass' : undeclared identifier
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(249) : error C2146: syntax error : missing ';' before identifier 'myClass'
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(249) : error C2065: 'myClass' : undeclared identifier
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(250) : error C2065: 'myClass' : undeclared identifier
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(250) : error C2228: left of '.Operation' must have class/struct/union
1> type is ''unknown-type''
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(253) : error C2065: 'Rename1' : undeclared identifier
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(253) : error C2146: syntax error : missing ';' before identifier 'test'
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(253) : error C2065: 'test' : undeclared identifier
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(255) : error C2065: 'test' : undeclared identifier
1>f:\\code\\d3dgraphicspipeline\\trunk\\refactortest\\refactortest\\rename.cpp(255) : error C2228: left of '.Operation' must have class/struct/union
1> type is ''unknown-type''
1>Rename2.cpp
1>Generating Code...
1>Build log was saved at "file://f:\\Code\\d3dgraphicspipeline\\trunk\\RefactorTest\\RefactorTest\\Debug\\BuildLog.htm"
1>RefactorTest - 12 error(s), 0 warning(s)
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
For instance, in the first error, it renamed the c'tor for the nested class by the same name in a namespace, but didn't rename the class itself:
class Rename1
{
public:
Rename1zz() : _y(0)
{
}
// #TEST#: R8 rename this method
int Operation()
{
return 0;
}
private:
void Method()
{
int _y = -4;
// #TEST#: R31 rename _y, which shadows member _y
_y = 6;
}
int _y;
};
The reuse of the same identifier in different, possibly shadowing, scopes is intentional. There are areas of C++ programming where the canonical forms are replete with repeated names.
Consider the use of the identifier value_type in template classes, for instance. If I'm renaming the identifier value_type in a template class T, VAX should show me the instances it *knows* are relevant to that class T, not every occurrence of the identifier "value_type" in the entire solution. |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on Feb 11 2010 3:52:30 PM |
|
|
|
feline
Whole Tomato Software
United Kingdom
19021 Posts |
Posted - Feb 15 2010 : 3:11:38 PM
|
We are using the same words to mean different things.
The class inside the namespace is clearly a different symbol. No references to that class should have been found and renamed. So to me that is finding to many references, it is not missing references.
Generally VA tries very hard to work out what is the correct references to find and update, but I am not claiming it is perfect. Sometimes C++ is a hard language to read. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Feb 15 2010 : 8:34:23 PM
|
quote:
Originally posted by feline
The class inside the namespace is clearly a different symbol. No references to that class should have been found and renamed. So to me that is finding to many references, it is not missing references.
I agree with that.
C++ is non-trivial to parse. The test suite is designed to hit the hard cases. |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on Feb 15 2010 8:34:44 PM |
|
|
| |
Topic |
|
Test suite available
