Skip to content

Enforce unique result in list-fix-paths #687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

TobiasNx
Copy link
Contributor

@TobiasNx TobiasNx commented Apr 25, 2025

Resolves #686.

@dr0i fixed the error could you help me with adding a test.

@dr0i
Copy link
Member

dr0i commented Apr 25, 2025

@TobiasNx look at ther error:

Execution failed for task ':metafix:checkstyleMain'.
A failure occurred while executing org.gradle.api.plugins.quality.internal.CheckstyleAction
Checkstyle rule violations were found. See the report at: file:///home/runner/work/metafacture-core/metafacture-core/metafix/build/reports/checkstyle/main.html
Checkstyle files with violations: 1
Checkstyle violations by severity: [error:4]

You may just have put some extra spaces or somethin along the line.

@dr0i dr0i assigned TobiasNx and unassigned dr0i Apr 25, 2025
@TobiasNx TobiasNx assigned dr0i and unassigned TobiasNx Apr 30, 2025
@TobiasNx
Copy link
Contributor Author

Thanks to @blackwinter the checkstyle is not failing anymore. @dr0i can you help me with tests?

@TobiasNx
Copy link
Contributor Author

TobiasNx commented Jun 20, 2025

@dr0i you changed the option here: 53c2e6b#diff-169b6a53d763d640ae1f135fea90ead61c4f2219219ab9539d8c21d0390aeebcR62

This makes the result no fix paths anymore. You could not use it anymore in a fix transformation, we need the index position for that. e.g. c.2 is a fix path, but c is only the path to the array, you could not use copy_field("c","new_field") to only copy this value, you would use copy_field("c.2","new_field") to only copy the specific value.

PS: This also puts two changes into one commit that should have been two commits.

@TobiasNx TobiasNx assigned dr0i and unassigned dr0i Jun 20, 2025
@dr0i dr0i force-pushed the 686-findFixValuesUniq branch from e0077cc to d5ff80d Compare June 20, 2025 13:39
@dr0i
Copy link
Member

dr0i commented Jun 20, 2025

I've modified the test, see d5ff80d.
If it's good the commits can be squashed.

@dr0i dr0i assigned TobiasNx and unassigned dr0i Jun 20, 2025
@dr0i dr0i moved this to Review in Metafacture Jun 20, 2025
@TobiasNx
Copy link
Contributor Author

TobiasNx commented Jun 20, 2025

I think I understand how to adjust the test now. Can I add another record to the test like this. Duplicate elements are already tested. I need to test duplicate field/values in multiple records. See: #686 (comment)

    private void processRecord() {
        finder.setReceiver(receiver);
        finder.startRecord("1");
        finder.literal("a", "An ETL test");
        finder.literal("b", "");
        finder.literal("b", "Dummi");
        finder.literal("b", "Dog");
        finder.literal("c", "");
        finder.literal("c", "ETL what?");
        finder.endRecord();
        finder.startRecord("2");
        finder.literal("a", "An another test");
        finder.literal("b", "");
        finder.literal("b", "Dummi");
        finder.literal("b", "Dog");
        finder.literal("c", "");
        finder.literal("c", "ETL what?");
        finder.endRecord();
        finder.closeStream();
    }
``

@TobiasNx TobiasNx assigned dr0i and unassigned TobiasNx Jun 20, 2025
@dr0i
Copy link
Member

dr0i commented Jun 20, 2025

you can add the test - why shouldn't you?

@dr0i dr0i assigned TobiasNx and unassigned dr0i Jun 20, 2025
@TobiasNx TobiasNx assigned dr0i and unassigned TobiasNx Jun 20, 2025
@TobiasNx
Copy link
Contributor Author

Done

@TobiasNx TobiasNx marked this pull request as ready for review June 20, 2025 14:36
@dr0i dr0i force-pushed the 686-findFixValuesUniq branch from cdaa04e to 9956282 Compare June 20, 2025 15:02
@dr0i dr0i force-pushed the 686-findFixValuesUniq branch from 9956282 to 6a4f548 Compare June 20, 2025 15:10
@dr0i dr0i assigned TobiasNx and unassigned dr0i Jun 20, 2025
@dr0i dr0i changed the title Add draft for bug fix for list-fix-paths #686 Add for bug fix for list-fix-paths #686 Jun 23, 2025
@dr0i dr0i changed the title Add for bug fix for list-fix-paths #686 Enforce unique result in list-fix-paths Jun 23, 2025
@dr0i
Copy link
Member

dr0i commented Jun 23, 2025

As discussed: readd the Mockito test and adapt the test to be successful, then assign @blackwinter for review.

@dr0i dr0i assigned dr0i and unassigned TobiasNx Jun 23, 2025
@dr0i dr0i moved this from Review to Working in Metafacture Jun 23, 2025
@fsteeg fsteeg assigned fsteeg and unassigned dr0i Jul 14, 2025
fsteeg added 2 commits July 14, 2025 12:29
Duplicate field was consecutive by coincidence, since all fields
in between were filtered. With added result field we need to sort.
@fsteeg fsteeg requested a review from blackwinter July 14, 2025 12:06
@fsteeg fsteeg assigned blackwinter and unassigned fsteeg Jul 14, 2025
@fsteeg fsteeg moved this from Working to Review in Metafacture Jul 14, 2025
@blackwinter blackwinter assigned TobiasNx and unassigned blackwinter Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

find-fix-paths does not uniq result
4 participants