Re: Idea: Remove PowerMock

2023-07-10 Thread Attila Turoczy
+1 Kill it! :)
mockito is a more modern approach. I think it is cool that we modernize our
platform, and remove old and unsupported tools and components.


On Mon, Jul 10, 2023 at 5:36 PM Ayush Saxena  wrote:

> +1, PowerMock as far as I remember has issues with JDK-11+ as well,
> one such ref :
> https://stackoverflow.com/questions/52966897/powermock-java-11
>
> -Ayush
>
> On Mon, 10 Jul 2023 at 20:18, Zsolt Miskolczi 
> wrote:
> >
> > Hi,
> >
> > Hive heavily uses PowerMock . The main
> > purpose of it is having static mocking.
> >
> > The sad thing is it seems PowerMock is dead:
> > - The main branch got it's lot commit in 2022 and and most of the
> > contributions last year were simple dependency upgrades:
> > https://github.com/powermock/powermock/commits/release/2.x
> > - The last release was in 2020
> > - And their mailing list looks dead as well. That is the last email on
> that
> > list: https://groups.google.com/g/powermock/c/JdYY3naZlbU. It asked if
> it
> > was discontinued and didn't get an answer at all.
> >
> > So officially, it is not dead but it seems it is.
> >
> > Back then when PowerMock development started, there were no static
> mocking
> > in mockito. But since then, it is possible using mockito-inline.
> >
> > I won't lie, it is hard to switch from PowerMock: it enables some coding
> > patterns that are considered bad patterns and it leads to code that is
> > harder to test. Last year I played with it and removed it from the
> > hive-exec module: https://github.com/apache/hive/pull/3798.
> >
> > The hard part in removing it is that PowerMock and mockito-inline don't
> > work together. So when we want to remove it, we have to do it in one pull
> > request for a given module. It cannot be separated into smaller steps.
> > The good news is as it relates to testing, pre commit tests can validate
> > the refactor.
> >
> > What do you think? Should we move away from PowerMock or keep it as it
> is?
> >
> > Thank you,
> > Zsolt Miskolczi
>


Re: Is there a way to test schema evolution scripts?

2023-07-10 Thread Stamatis Zampetakis
Hello,

The tests under DbInstallBase [1] seem to do more or less what you
want. Check them out and if something is missing don't hesitate to
enrich them.

Best,
Stamatis

[1] 
https://github.com/apache/hive/blob/5e46e80bc7d059093aece81e3886ba5ee425ee95/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java

On Mon, Jul 10, 2023 at 5:46 PM Ayush Saxena  wrote:
>
> Hi Zsolt,
> You can start by exploring this:
> https://github.com/apache/hive/blob/master/standalone-metastore/DEV-README
>
> It has steps to run those metatore scripts over different database
> types locally.
>
> -Ayush
>
> On Mon, 10 Jul 2023 at 20:30, Zsolt Miskolczi  
> wrote:
> >
> > Hi,
> >
> > I just stared at a pull request about using liquibase for schema evolution
> > and I was thinking how such a change can be validated.
> >
> > I think the main issue is that we support multiple types of relational
> > databases but precommit tests don't test them at all.
> >
> > I think if we want to check if the idea works, it is enough to:
> > - introduce a proper docker based instance for each type of database
> > (postgres and derby first).
> > - run the upgrade scripts on them
> > - check if any error occurs
> >
> > And the end goal should be something like having validation scripts and
> > check if we are able to run selects after the upgrades and fresh installs.
> >
> >
> > What do you think about that?
> >
> > Thank you,
> > Zsolt Miskolczi


Re: Is there a way to test schema evolution scripts?

2023-07-10 Thread Ayush Saxena
Hi Zsolt,
You can start by exploring this:
https://github.com/apache/hive/blob/master/standalone-metastore/DEV-README

It has steps to run those metatore scripts over different database
types locally.

-Ayush

On Mon, 10 Jul 2023 at 20:30, Zsolt Miskolczi  wrote:
>
> Hi,
>
> I just stared at a pull request about using liquibase for schema evolution
> and I was thinking how such a change can be validated.
>
> I think the main issue is that we support multiple types of relational
> databases but precommit tests don't test them at all.
>
> I think if we want to check if the idea works, it is enough to:
> - introduce a proper docker based instance for each type of database
> (postgres and derby first).
> - run the upgrade scripts on them
> - check if any error occurs
>
> And the end goal should be something like having validation scripts and
> check if we are able to run selects after the upgrades and fresh installs.
>
>
> What do you think about that?
>
> Thank you,
> Zsolt Miskolczi


Re: Idea: Remove PowerMock

2023-07-10 Thread Ayush Saxena
+1, PowerMock as far as I remember has issues with JDK-11+ as well,
one such ref : https://stackoverflow.com/questions/52966897/powermock-java-11

-Ayush

On Mon, 10 Jul 2023 at 20:18, Zsolt Miskolczi  wrote:
>
> Hi,
>
> Hive heavily uses PowerMock . The main
> purpose of it is having static mocking.
>
> The sad thing is it seems PowerMock is dead:
> - The main branch got it's lot commit in 2022 and and most of the
> contributions last year were simple dependency upgrades:
> https://github.com/powermock/powermock/commits/release/2.x
> - The last release was in 2020
> - And their mailing list looks dead as well. That is the last email on that
> list: https://groups.google.com/g/powermock/c/JdYY3naZlbU. It asked if it
> was discontinued and didn't get an answer at all.
>
> So officially, it is not dead but it seems it is.
>
> Back then when PowerMock development started, there were no static mocking
> in mockito. But since then, it is possible using mockito-inline.
>
> I won't lie, it is hard to switch from PowerMock: it enables some coding
> patterns that are considered bad patterns and it leads to code that is
> harder to test. Last year I played with it and removed it from the
> hive-exec module: https://github.com/apache/hive/pull/3798.
>
> The hard part in removing it is that PowerMock and mockito-inline don't
> work together. So when we want to remove it, we have to do it in one pull
> request for a given module. It cannot be separated into smaller steps.
> The good news is as it relates to testing, pre commit tests can validate
> the refactor.
>
> What do you think? Should we move away from PowerMock or keep it as it is?
>
> Thank you,
> Zsolt Miskolczi


Is there a way to test schema evolution scripts?

2023-07-10 Thread Zsolt Miskolczi
Hi,

I just stared at a pull request about using liquibase for schema evolution
and I was thinking how such a change can be validated.

I think the main issue is that we support multiple types of relational
databases but precommit tests don't test them at all.

I think if we want to check if the idea works, it is enough to:
- introduce a proper docker based instance for each type of database
(postgres and derby first).
- run the upgrade scripts on them
- check if any error occurs

And the end goal should be something like having validation scripts and
check if we are able to run selects after the upgrades and fresh installs.


What do you think about that?

Thank you,
Zsolt Miskolczi


Idea: Remove PowerMock

2023-07-10 Thread Zsolt Miskolczi
Hi,

Hive heavily uses PowerMock . The main
purpose of it is having static mocking.

The sad thing is it seems PowerMock is dead:
- The main branch got it's lot commit in 2022 and and most of the
contributions last year were simple dependency upgrades:
https://github.com/powermock/powermock/commits/release/2.x
- The last release was in 2020
- And their mailing list looks dead as well. That is the last email on that
list: https://groups.google.com/g/powermock/c/JdYY3naZlbU. It asked if it
was discontinued and didn't get an answer at all.

So officially, it is not dead but it seems it is.

Back then when PowerMock development started, there were no static mocking
in mockito. But since then, it is possible using mockito-inline.

I won't lie, it is hard to switch from PowerMock: it enables some coding
patterns that are considered bad patterns and it leads to code that is
harder to test. Last year I played with it and removed it from the
hive-exec module: https://github.com/apache/hive/pull/3798.

The hard part in removing it is that PowerMock and mockito-inline don't
work together. So when we want to remove it, we have to do it in one pull
request for a given module. It cannot be separated into smaller steps.
The good news is as it relates to testing, pre commit tests can validate
the refactor.

What do you think? Should we move away from PowerMock or keep it as it is?

Thank you,
Zsolt Miskolczi