streampipes icon indicating copy to clipboard operation
streampipes copied to clipboard

Fix JUnit Parameterized Tests

Open tenthe opened this issue 11 months ago • 18 comments

Description

Due to the update from JUnit4 to JUnit5, we had to deactivate some tests that do not use the JUnit5 syntax for parameterized tests. Below you will find a list of these tests that should be migrated step by step.

To see an example, I have started to migrate a first test (see TestBooleanFilterProcessor).

If you are interested in working on one of the tests, please let us know and we will create an issue for each test and assign it to you.

List of classes

  • [ ] #2754
  • [ ] #2764
  • [ ] #2754
  • [x] #2771
  • [x] #2772
  • [x] #2773
  • [x] #2774
  • [ ] #2775
  • [x] #2545
  • [ ] #2776
  • [ ] #2777
  • [ ] #2778
  • [ ] #2779

tenthe avatar Mar 12 '24 09:03 tenthe

Hi, i want to work on this issue. TestBooleanCounterProcessor Could you assign this issue?

nine03 avatar Mar 13 '24 07:03 nine03

Hi @nine03, Thank you for your interest. I have created an issue for you. Can you please comment under it so that I can assign it to you. If you have any questions, please let us know.

Cheers, Philipp

tenthe avatar Mar 13 '24 07:03 tenthe

currently looking at TestSizeMeasureProcessor what are the values for these parameters @Parameterized.Parameter public String sizeUnit;

@Parameterized.Parameter(1) public int numOfBytes;

@Parameterized.Parameter(2) public double expectedSize;

@Parameterized.Parameter(3) public double allowableError;

pambrocio avatar Apr 21 '24 00:04 pambrocio

Is it ok to use enums for this?? do we just need paremeterized inputs for org.apache.streampipes.processors.enricher.jvm.processor.sizemeasure.TestSizeMeasureProcessor#testSizeMeasureProcessor, (this is currently commented) . And Just make the test work???

pambrocio avatar Apr 21 '24 13:04 pambrocio

would like to take on TestStringTimerProcessor

pambrocio avatar Apr 23 '24 11:04 pambrocio

Here you go: https://github.com/apache/streampipes/issues/2764 🙂

bossenti avatar Apr 23 '24 13:04 bossenti

please assign the remaining Test classes to me

pambrocio avatar Apr 24 '24 02:04 pambrocio

@pambrocio issues are created (see at the top) I can assign you once you wrote a comment there

bossenti avatar Apr 24 '24 06:04 bossenti

Thanks @bossenti

pambrocio avatar Apr 24 '24 07:04 pambrocio

Hey @pambrocio,

I just realized that some tests you have created PRs for are also part of https://github.com/apache/streampipes/pull/2375 (for reference: https://github.com/apache/streampipes/issues/2737, https://github.com/apache/streampipes/issues/2374). Would be great if we could align both endeavors to avoid extra work. Sorry for not pointing out earlier.

bossenti avatar Apr 25 '24 06:04 bossenti

Hi @bossenti ,

Too bad i am almost done only one class left. Shall i continue?? this is the remaining class i have not migrated https://github.com/apache/streampipes/issues/2779 TestStringToStateProcessor

pambrocio avatar Apr 25 '24 07:04 pambrocio

@tenthe

Any advice on this?

pambrocio avatar Apr 25 '24 07:04 pambrocio

Hi @tenthe @bossenti @IsaakKrut,

the remaining classes have been migrated to Junit 5, Tell me what needs to be done to get my work merged to the main branch.

Thanks, Pambrocio

pambrocio avatar Apr 25 '24 07:04 pambrocio

Hi @bossenti ,

Too bad i am almost done only one class left. Shall i continue?? this is the remaining class i have not migrated #2779 TestStringToStateProcessor

Actually, I already mentioned that earlier: https://github.com/apache/streampipes/issues/2754#issuecomment-2069513982

Would you be interested to contribute to #2375 by adapting your migrated tests to the framework introduced there and help to improve it?

If not, we can merge the migrated tests that are not part of #2375. Otherwise, we would cause a lot of migration effort in #2375.

bossenti avatar Apr 25 '24 08:04 bossenti

Hi @bossenti,

Would love to. Let me check the discussion

pambrocio avatar Apr 25 '24 08:04 pambrocio

@pambrocio would be great if you are willing to continue your work based on https://github.com/apache/streampipes/pull/2375#issuecomment-2083177795 🙂

bossenti avatar May 02 '24 07:05 bossenti

@bossenti, I am quite unclerar on what code changes need to be implemented

pambrocio avatar May 02 '24 09:05 pambrocio

In #2375 we introduced a standardized approach how pipeline elements can (and should) be tested in future (https://github.com/apache/streampipes/pull/2375/files#diff-a8a7ae4c9654c4422ef00768c7887a92bfffc6e0726c1922531f57ae865ee531). The PR also contains some examples how this approach can be applied, e.g., https://github.com/apache/streampipes/pull/2375/files#diff-49a3517e7b83d0021ebf95c11e120d451297aff1c3eea3aab3f43dc8042cf9f0

So the task now is to update your PRs accordingly. This includes updating your branch with the recent changes in the dev branch and change the implementation of the tests so that they use the above described approach. As @tenthe already said, this approach is not fixed yet, so if you have any feedback, improvements we are happy to hear and address them or you can directly improve the proposed approach.

Does that make it clearer? 🙂

bossenti avatar May 02 '24 10:05 bossenti