On 08/11/2011 11:42 PM, Ganath Rathnayake wrote:
> Hi Adam,
>
> Thanks for your ideas and comments. I like to do some the changes you
> mentioned.
>
>
>> Your script should do 'set -e' at the beginning. It's possible that
>> your script is buggy, or doesn't handle something on a particular
>> machine, or the disk runs out of space. Without that, you would have
>> to check the return of *every* command(cp, rm, cat, whatever), and
>> handle the error. Without such checking, things can sometimes go very
>> bad.
>>
>
> I'll do it.
>
>
>>
>>> 1. Creating the main/java and test/java folders in the src directory and
>> put
>>> test files in test/java and rest in the main/java
>>> I did this by putting the tests in the test folders in each module in the
>>> test/java folder. I used the following code to do that. $test variable is
>>> the folder structure of each module.
>>
>> What is $test? Don't summarize, show code. I don't understand your
>> explanation.
>>
>
> $test is a variable I use to store the relative path to the test folders. I
> used this variable since one module can have several different test classes.
I think I know what you mean, but if so, then are you implying that
you call the script manually, changing $test each time? If so, don't
do that. All logic should be contained in the script. Even if you
have to hard-code the list of things being iterated.
There are other people on this list who could then help remove the
hard-coded sub-package list, and extract it directly from the filesystem.
The following is completely untested. It moves all files in
src/**/test/* to src/test/java, then moves everything left to
src/main/java.
It does *NOT* handle things in src/META-INF/services that are actually
located in src/**/test/*. It does *NOT* handle extracting the list of
*all* test package names; but if you don't change the package names,
then you probably don't need that.
Fixing META-INF/services involves finding all the files that reference
*/test/*, making a copy of that file in test, remove the lines that
are in main, leaving only the test versions, then modifying the
original to remove the test versions. That is an excersize left to
the reader.
==
for comp_xml in
framework/*/ofbiz-component.xml
applications/*/ofbiz-component.xml
specialpurpose/*/ofbiz-component.xml
do
comp_dir="${comp_xml%/ofbiz-component_xml}"
if ! [ -d "$comp_dir/src" ]; then
continue
fi
for test_subdir in
$(find "$comp_dir/src" -type d -name test -printf '%P\n' | sort -r)
do
if ! [ -d "$comp_dir/src/test/java/$test_subdir" ]; then
mkdir -p "$comp_dir/src/test/java/$test_subdir"
fi
for file in
$(find "$comp_dir/src/$test_subdir" -maxdepth 1 \
-type f -printf '%P'n')
do
svn mv \
"$comp_dir/src/$file" \
"$comp_dir/src/test/java/$file"
done
done
for file in
$(find "$comp_dir/src" -type f -printf '%P\n')
do
svn mv \
"$comp_dir/src/$file" \
"$comp_dir/src/main/java/$file"
done
done
==
ps: The script is mostly POSIX, except for:
1: If $(find) returns an empty list, then non-bash shells will throw a
syntax error.
2: I am not certain if find/%P is POSIX.
3: ${%} actually IS POSIX, most people just aren't aware of it.
4: ! [ -test file ] is POSIX, [ ! -test file ] isn't.
5: [ arg op value ] && [ arg op value ] is POSIX, [ arg op value -a
arg op value ] isn't. (I don't use this construct here).
6: mkdir -p isn't POSIX, but I was too lasy to do it the right away.
>
>
>>
>>> cp main/java/$test/test/* test/java/$test/
>>
>> cp -a, preserve timestamps/permissions/ownership.
>>
>> Your cp will *not* work if there are sub-folders in test, as it would
>> then copy the .svn files, which is *not* what you want.
>>
>> rsync main/java/$test/test/ test/java/$test/ -a --exclude '.svn'
>>
>
> Since I worked in the the git repo I didn't notice that. Thanks for pointing
> out me the issue and the solution.
That's ok initially, but this kind of script should *not* be run in
git, once it is fully developed. The script should *not* do:
cp foo bar
or
mv foo bar
Those will not maintain proper svn history. The script should
directly call:
mkdir $new/path/directory
svn add new/path/directory
svn cp old/path/directory/foo new/path/directory/bar
or svn mv. Otherwise, you won't maintain proper svn history.
> sed 's,@{prefix}src,@{prefix}src/main/java,'
>>
>> Note the use of ',' as the separator; this is easier to read, as you
>> don't have to escape all the / in the path.
>>
>
> Thanks for the suggestion. I didn't know it before.
Actually, I think you did, I seem to recall other sed uses in your
script that do change the separator(from / to #).
>>> cat temp >macros.xml
>>> rm temp
>>>
>>> 3. Fixing the paths in build.xml files. This was achieved by first
>> replacing
>>> all the paths by adding main/java/ to the baginning of each path and then
>>> changing it with test/java if it is a test. You can see the code follows.
>>> Also some package name the files also change when it needed to do so.
>>>
>>> for x in $(ls main/java/$test/test/)
>>> do
>>> echo $x
>>> sed 's/\.test;/;/' <main/java/$test/test/$x >temp
>>>
>>> cat temp >main/java/$test/test/$x
>>>
>>> j=`expr "$test" : 'org/ofbiz/base'`
>>> if [ $j != 0 ];
>>> then
>>> sed 's/\.test\./\./' <main/java/$test/test/$x >temp
>>> cat temp >main/java/$test/test/$x
>>> fi
>>
>> What? Are we removing the test sub packages? As in:
>>
>> package org.ofbiz.entity.test;
>>
>> becomes
>>
>> package org.ofbiz.entity;
>>
>> ?
>>
>> If so, then NO! DO NOT DO THAT. Test cases should not ever be inside
>> the same package as the code they are testing. Bugs might be missed,
>> as you can call package private/protected methods when in the same
>> package, that you can't when in another sub-package.
>>
>> I can *not* stress this enough. Do *NOT* move the test code out of a
>> test sub-package.
>>
>>
> Here I removed the test folder because it already in a different package.
> src
> |---main
> | `----java
> `---test
> `----java
>
> Since all the test classes in the test/java I thought we do not have to use
> extra test class for module structure. We can make a jar for that by
> changing the packaging criteria described in macro.xml. Please let me know
> your ideas on this.
Just because the prefix for the test class location is different, does
not mean we can remove the actual 'test' from the package name.
Otherwise:
src/main/java:
org/ofbiz/entity/GenericDelegator.java
src/test/java:
org/ofbiz/entity/GenericDelegatorTest.java
If those are put into 2 different jars, and the jars are 'signed' and
'sealed', then the second jar is not allowed to put classes into a
package that was previously sealed. I can see us wanting to do that.
Also, if there are package private or protected methods in
GenericDelegator, then having the test code in the same package might
allow for accidentally calling those internal methods; that's not a
proper test case. You should test the public api(by instantiating the
object and calling it's methods), or the protected api, by extending
the class(and then calling the protected methods).
Having a separate 'test' sub package makes both of these possible, and
leads to better test cases. The test cases also end up not being tied
to the implementation, so that you can do a heavy rewrite to the
class, moving methods around, and as long as you keep the same public
api, things should still work.
