You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@dubbo.apache.org by yuhang xiu <ca...@apache.org> on 2018/10/09 06:49:01 UTC
New module for serialization test
Hi, community.
Today I found that the unit testing of dubbo's serialization module has
serious code duplication problems, mainly on several pojo classes, such as
BigPerson, PersonInfo, and so on. These classes even appear in the
dubbo-common module.
I think this repeated POJO class is not necessary, so I will start to
concentrate the unit testing of the serialization module into the
dubbo-serialization-test module, which will be a brand new module belonging
to dubbo-serialization. What do you think?
related issue:
https://github.com/apache/incubator-dubbo/issues/2623
Re: New module for serialization test
Posted by Ian Luo <ia...@gmail.com>.
awesome, now we have higher code coverage. Thank you guys :)
-Ian.
On Mon, Oct 15, 2018 at 11:31 AM yuhang xiu <ca...@gmail.com> wrote:
> Hi, all
> After @Yunkun Huang's pr[1], the current UT coverage is correct, please
> review pr[2] again.
> Thx for @Yunkun Huang and @Xin Wang's help for UT coverage.
>
> [1] https://github.com/apache/incubator-dubbo/pull/2643
> [2] https://github.com/apache/incubator-dubbo/pull/2632
>
> yuhang xiu <ca...@gmail.com> 于2018年10月11日周四 下午2:40写道：
>
> >
> > Hi, community.
> >
> > After I completed this PR, @Xin Wang and @YunKun Huang reminded me that
> in
> > the codecov report, UT coverage dropped by 0.58%, which is not a small
> > number.
> >
> > But after @YunKun Huang's review and my second check, we didn't reduce
> any
> > single-test examples.
> >
> > All I did was delete some of the extra classes in the TEST module, which
> > in a strict sense would not cause the UT coverage to drop.
> >
> > I am still on the official website of codecov [1], and I have not found a
> > suitable answer yet. Does anyone else have a similar experience? How did
> > you solve this problem?
> >
> > [1] https://docs.codecov.io/docs
> >
> > yuhang xiu <ca...@gmail.com> 于2018年10月10日周三 下午4:06写道：
> >
> >> Hi, jerrick
> >>
> >> At present, I don't think this universal test module is necessary.
> >>
> >> The reason is that in general, only the serialization module has more
> >> requirements for the POJO class, and the test modules of other modules
> are
> >> basically not needed.
> >>
> >> I will continue to pay attention to the use and reuse of the POJO class.
> >> If dubbo really comes to a stage that has to be integrated, I will
> solve it.
> >> :)
> >>
> >> the pr is here:
> >> https://github.com/apache/incubator-dubbo/pull/2632
> >>
> >> I have finish it and pass the ci check. Can anyone help me with a
> review?
> >>
> >> Jerrick Zhu <je...@apache.org> 于2018年10月10日周三 下午1:58写道：
> >>
> >>> How about a dubbo-test-common ?
> >>>
> >>> Maybe other modules also need these POJOs.
> >>>
> >>> On Tue, Oct 9, 2018 at 10:04 PM Kun Song <so...@gmail.com> wrote:
> >>>
> >>> > I think a single test module would be helpful, otherwise every
> >>> > serialization module will have to keep a duplicate copy.
> >>> >
> >>> > And I recently encountered this problem when support Protobuf
> >>> > serialization: https://github.com/apache/incubator-dubbo/pull/2618 <
> >>> > https://github.com/apache/incubator-dubbo/pull/2618>.
> >>> >
> >>> > Looking forward it!
> >>> >
> >>> > > 在 2018年10月9日，下午2:49，yuhang xiu <ca...@apache.org> 写道：
> >>> > >
> >>> > > Hi, community.
> >>> > >
> >>> > > Today I found that the unit testing of dubbo's serialization module
> >>> has
> >>> > > serious code duplication problems, mainly on several pojo classes,
> >>> such
> >>> > as
> >>> > > BigPerson, PersonInfo, and so on. These classes even appear in the
> >>> > > dubbo-common module.
> >>> > >
> >>> > > I think this repeated POJO class is not necessary, so I will start
> to
> >>> > > concentrate the unit testing of the serialization module into the
> >>> > > dubbo-serialization-test module, which will be a brand new module
> >>> > belonging
> >>> > > to dubbo-serialization. What do you think?
> >>> > >
> >>> > > related issue:
> >>> > > https://github.com/apache/incubator-dubbo/issues/2623
> >>> >
> >>> >
> >>>
> >>
>
Re: New module for serialization test
Posted by yuhang xiu <ca...@gmail.com>.
Hi, all
After @Yunkun Huang's pr[1], the current UT coverage is correct, please
review pr[2] again.
Thx for @Yunkun Huang and @Xin Wang's help for UT coverage.
[1] https://github.com/apache/incubator-dubbo/pull/2643
[2] https://github.com/apache/incubator-dubbo/pull/2632
yuhang xiu <ca...@gmail.com> 于2018年10月11日周四 下午2:40写道：
>
> Hi, community.
>
> After I completed this PR, @Xin Wang and @YunKun Huang reminded me that in
> the codecov report, UT coverage dropped by 0.58%, which is not a small
> number.
>
> But after @YunKun Huang's review and my second check, we didn't reduce any
> single-test examples.
>
> All I did was delete some of the extra classes in the TEST module, which
> in a strict sense would not cause the UT coverage to drop.
>
> I am still on the official website of codecov [1], and I have not found a
> suitable answer yet. Does anyone else have a similar experience? How did
> you solve this problem?
>
> [1] https://docs.codecov.io/docs
>
> yuhang xiu <ca...@gmail.com> 于2018年10月10日周三 下午4:06写道：
>
>> Hi, jerrick
>>
>> At present, I don't think this universal test module is necessary.
>>
>> The reason is that in general, only the serialization module has more
>> requirements for the POJO class, and the test modules of other modules are
>> basically not needed.
>>
>> I will continue to pay attention to the use and reuse of the POJO class.
>> If dubbo really comes to a stage that has to be integrated, I will solve it.
>> :)
>>
>> the pr is here:
>> https://github.com/apache/incubator-dubbo/pull/2632
>>
>> I have finish it and pass the ci check. Can anyone help me with a review?
>>
>> Jerrick Zhu <je...@apache.org> 于2018年10月10日周三 下午1:58写道：
>>
>>> How about a dubbo-test-common ?
>>>
>>> Maybe other modules also need these POJOs.
>>>
>>> On Tue, Oct 9, 2018 at 10:04 PM Kun Song <so...@gmail.com> wrote:
>>>
>>> > I think a single test module would be helpful, otherwise every
>>> > serialization module will have to keep a duplicate copy.
>>> >
>>> > And I recently encountered this problem when support Protobuf
>>> > serialization: https://github.com/apache/incubator-dubbo/pull/2618 <
>>> > https://github.com/apache/incubator-dubbo/pull/2618>.
>>> >
>>> > Looking forward it!
>>> >
>>> > > 在 2018年10月9日，下午2:49，yuhang xiu <ca...@apache.org> 写道：
>>> > >
>>> > > Hi, community.
>>> > >
>>> > > Today I found that the unit testing of dubbo's serialization module
>>> has
>>> > > serious code duplication problems, mainly on several pojo classes,
>>> such
>>> > as
>>> > > BigPerson, PersonInfo, and so on. These classes even appear in the
>>> > > dubbo-common module.
>>> > >
>>> > > I think this repeated POJO class is not necessary, so I will start to
>>> > > concentrate the unit testing of the serialization module into the
>>> > > dubbo-serialization-test module, which will be a brand new module
>>> > belonging
>>> > > to dubbo-serialization. What do you think?
>>> > >
>>> > > related issue:
>>> > > https://github.com/apache/incubator-dubbo/issues/2623
>>> >
>>> >
>>>
>>
Re: New module for serialization test
Posted by yuhang xiu <ca...@gmail.com>.
Hi, community.
After I completed this PR, @Xin Wang and @YunKun Huang reminded me that in
the codecov report, UT coverage dropped by 0.58%, which is not a small
number.
But after @YunKun Huang's review and my second check, we didn't reduce any
single-test examples.
All I did was delete some of the extra classes in the TEST module, which in
a strict sense would not cause the UT coverage to drop.
I am still on the official website of codecov [1], and I have not found a
suitable answer yet. Does anyone else have a similar experience? How did
you solve this problem?
[1] https://docs.codecov.io/docs
yuhang xiu <ca...@gmail.com> 于2018年10月10日周三 下午4:06写道：
> Hi, jerrick
>
> At present, I don't think this universal test module is necessary.
>
> The reason is that in general, only the serialization module has more
> requirements for the POJO class, and the test modules of other modules are
> basically not needed.
>
> I will continue to pay attention to the use and reuse of the POJO class.
> If dubbo really comes to a stage that has to be integrated, I will solve it.
> :)
>
> the pr is here:
> https://github.com/apache/incubator-dubbo/pull/2632
>
> I have finish it and pass the ci check. Can anyone help me with a review?
>
> Jerrick Zhu <je...@apache.org> 于2018年10月10日周三 下午1:58写道：
>
>> How about a dubbo-test-common ?
>>
>> Maybe other modules also need these POJOs.
>>
>> On Tue, Oct 9, 2018 at 10:04 PM Kun Song <so...@gmail.com> wrote:
>>
>> > I think a single test module would be helpful, otherwise every
>> > serialization module will have to keep a duplicate copy.
>> >
>> > And I recently encountered this problem when support Protobuf
>> > serialization: https://github.com/apache/incubator-dubbo/pull/2618 <
>> > https://github.com/apache/incubator-dubbo/pull/2618>.
>> >
>> > Looking forward it!
>> >
>> > > 在 2018年10月9日，下午2:49，yuhang xiu <ca...@apache.org> 写道：
>> > >
>> > > Hi, community.
>> > >
>> > > Today I found that the unit testing of dubbo's serialization module
>> has
>> > > serious code duplication problems, mainly on several pojo classes,
>> such
>> > as
>> > > BigPerson, PersonInfo, and so on. These classes even appear in the
>> > > dubbo-common module.
>> > >
>> > > I think this repeated POJO class is not necessary, so I will start to
>> > > concentrate the unit testing of the serialization module into the
>> > > dubbo-serialization-test module, which will be a brand new module
>> > belonging
>> > > to dubbo-serialization. What do you think?
>> > >
>> > > related issue:
>> > > https://github.com/apache/incubator-dubbo/issues/2623
>> >
>> >
>>
>
Re: New module for serialization test
Posted by yuhang xiu <ca...@gmail.com>.
Hi, jerrick
At present, I don't think this universal test module is necessary.
The reason is that in general, only the serialization module has more
requirements for the POJO class, and the test modules of other modules are
basically not needed.
I will continue to pay attention to the use and reuse of the POJO class. If
dubbo really comes to a stage that has to be integrated, I will solve it.
:)
the pr is here:
https://github.com/apache/incubator-dubbo/pull/2632
I have finish it and pass the ci check. Can anyone help me with a review?
Jerrick Zhu <je...@apache.org> 于2018年10月10日周三 下午1:58写道：
> How about a dubbo-test-common ?
>
> Maybe other modules also need these POJOs.
>
> On Tue, Oct 9, 2018 at 10:04 PM Kun Song <so...@gmail.com> wrote:
>
> > I think a single test module would be helpful, otherwise every
> > serialization module will have to keep a duplicate copy.
> >
> > And I recently encountered this problem when support Protobuf
> > serialization: https://github.com/apache/incubator-dubbo/pull/2618 <
> > https://github.com/apache/incubator-dubbo/pull/2618>.
> >
> > Looking forward it!
> >
> > > 在 2018年10月9日，下午2:49，yuhang xiu <ca...@apache.org> 写道：
> > >
> > > Hi, community.
> > >
> > > Today I found that the unit testing of dubbo's serialization module has
> > > serious code duplication problems, mainly on several pojo classes, such
> > as
> > > BigPerson, PersonInfo, and so on. These classes even appear in the
> > > dubbo-common module.
> > >
> > > I think this repeated POJO class is not necessary, so I will start to
> > > concentrate the unit testing of the serialization module into the
> > > dubbo-serialization-test module, which will be a brand new module
> > belonging
> > > to dubbo-serialization. What do you think?
> > >
> > > related issue:
> > > https://github.com/apache/incubator-dubbo/issues/2623
> >
> >
>
Re: New module for serialization test
Posted by Jerrick Zhu <je...@apache.org>.
How about a dubbo-test-common ?
Maybe other modules also need these POJOs.
On Tue, Oct 9, 2018 at 10:04 PM Kun Song <so...@gmail.com> wrote:
> I think a single test module would be helpful, otherwise every
> serialization module will have to keep a duplicate copy.
>
> And I recently encountered this problem when support Protobuf
> serialization: https://github.com/apache/incubator-dubbo/pull/2618 <
> https://github.com/apache/incubator-dubbo/pull/2618>.
>
> Looking forward it!
>
> > 在 2018年10月9日，下午2:49，yuhang xiu <ca...@apache.org> 写道：
> >
> > Hi, community.
> >
> > Today I found that the unit testing of dubbo's serialization module has
> > serious code duplication problems, mainly on several pojo classes, such
> as
> > BigPerson, PersonInfo, and so on. These classes even appear in the
> > dubbo-common module.
> >
> > I think this repeated POJO class is not necessary, so I will start to
> > concentrate the unit testing of the serialization module into the
> > dubbo-serialization-test module, which will be a brand new module
> belonging
> > to dubbo-serialization. What do you think?
> >
> > related issue:
> > https://github.com/apache/incubator-dubbo/issues/2623
>
>
Re: New module for serialization test
Posted by Kun Song <so...@gmail.com>.
I think a single test module would be helpful, otherwise every serialization module will have to keep a duplicate copy.
And I recently encountered this problem when support Protobuf serialization: https://github.com/apache/incubator-dubbo/pull/2618 <https://github.com/apache/incubator-dubbo/pull/2618>.
Looking forward it!
> 在 2018年10月9日，下午2:49，yuhang xiu <ca...@apache.org> 写道：
>
> Hi, community.
>
> Today I found that the unit testing of dubbo's serialization module has
> serious code duplication problems, mainly on several pojo classes, such as
> BigPerson, PersonInfo, and so on. These classes even appear in the
> dubbo-common module.
>
> I think this repeated POJO class is not necessary, so I will start to
> concentrate the unit testing of the serialization module into the
> dubbo-serialization-test module, which will be a brand new module belonging
> to dubbo-serialization. What do you think?
>
> related issue:
> https://github.com/apache/incubator-dubbo/issues/2623