Skip to content

FINERACT-2421: Add unit tests for ExternalIdFactory #5276

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.

Sign up for GitHub

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

Jump to bottom
Merged
adamsaghy merged 2 commits into from
Jan 15, 2026
Merged

FINERACT-2421: Add unit tests for ExternalIdFactory #5276

adamsaghy merged 2 commits into from
Jan 15, 2026
+133 −0

Conversation

@airajena
Copy link
Contributor

@airajena airajena commented Dec 30, 2025

Description

This PR adds comprehensive unit tests for the ExternalIdFactory class in the fineract-core module.

Changes

  • Added ExternalIdFactoryTest.java with 8 test methods covering:
    • produce(String) - tests for valid, null, empty, and blank strings
    • produce(List<String>) - tests for valid lists, mixed values, null list, and empty list

Testing

  • Tests verify that:
    • Valid strings produce ExternalId with correct value
    • Null/empty/blank strings produce empty ExternalId
    • List factory method correctly converts all values
    • Null list throws NullPointerException as expected

Related Issue

FINERACT-2421

@adamsaghy
Copy link
Contributor

adamsaghy commented Jan 8, 2026 

> Task :fineract-loan:classes
/home/runner/work/fineract/fineract/fineract-core/src/test/java/org/apache/fineract/infrastructure/core/service/ExternalIdFactoryTest.java:49: error: reference to produce is ambiguous

@airajena
Copy link
Contributor Author

airajena commented Jan 9, 2026 

> Task :fineract-loan:classes
/home/runner/work/fineract/fineract/fineract-core/src/test/java/org/apache/fineract/infrastructure/core/service/ExternalIdFactoryTest.java:49: error: reference to produce is ambiguous

Fix pushed

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...t-provider/src/main/java/org/apache/fineract/portfolio/calendar/command/CalendarCommand.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...t-provider/src/main/java/org/apache/fineract/portfolio/calendar/command/CalendarCommand.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...apache/fineract/portfolio/calendar/serialization/CalendarCommandFromApiJsonDeserializer.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...apache/fineract/portfolio/calendar/serialization/CalendarCommandFromApiJsonDeserializer.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...apache/fineract/portfolio/calendar/serialization/CalendarCommandFromApiJsonDeserializer.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...apache/fineract/portfolio/calendar/serialization/CalendarCommandFromApiJsonDeserializer.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy reviewed Jan 12, 2026
View reviewed changes
...apache/fineract/portfolio/calendar/serialization/CalendarCommandFromApiJsonDeserializer.java
Copy link
Contributor

@adamsaghy adamsaghy Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant change for testing ExternalIdFactory. Please remove!

adamsaghy
adamsaghy requested changes Jan 12, 2026
View reviewed changes
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove not relevant changes!

airajena and others added 2 commits January 12, 2026 20:51
@airajena
FINERACT-2421: Add unit tests for ExternalIdFactory
418964e
@airajena
FINERACT-2421: Fix ambiguous method reference in ExternalIdFactoryTest
e38679b 
Cast null to String to resolve ambiguous method call between
produce(String) and produce(List<String>) overloads.
@airajena airajena force-pushed the FINERACT-2421/add-externalid-factory-tests branch from c7834eb to e38679b Compare January 13, 2026 16:14
@airajena
Copy link
Contributor Author

airajena commented Jan 14, 2026

Please remove not relevant changes!

Done

@adamsaghy
Copy link
Contributor

adamsaghy commented Jan 15, 2026

@airajena Please squash you commits next time!

@adamsaghy adamsaghy merged commit 3ba476f into apache:develop Jan 15, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamsaghy adamsaghy adamsaghy requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@airajena @adamsaghy