Hi all,

I have raised FINERACT-2781 [1] with a pull request [2] that adds a
documentation chapter for writing integration tests against the generated
Feign client, plus a Checkstyle rule to go with it. I would like feedback
before this is merged, particularly on the documentation.
Background

Over the last few months, as a GSoC contributor, I have been working on
Feign migration, and several pull requests have migrated integration tests
from REST Assured to the generated Feign client [4]. During those reviews,
the same points came up repeatedly: fix a missing model field in the
Swagger DTO instead of working around it in the test, assert the specific
error code rather than just that a call failed, do not weaken a
verification because the generated model lacks a field, and so on.

At the moment, that guidance exists only in pull request comments. A new
contributor has no way to find it, and reviewers have to repeat the same
explanations.
What the Pull Request Contains

   1.

   *A new chapter, "Feign Integration Tests":* Covers how a test reaches
   the client, how to assert failures, and what to do when a generated model
   is missing a field. The conventions are written as a numbered list (IT-01
   to IT-20) so a review comment can point directly to a rule instead of
   restating it.
   2.

   *A Checkstyle rule:* Rejects REST Assured imports in the
   integrationtests.client.feign package, which holds the base classes and
   helpers every migrated test inherits from. It reuses
   ForbidCertainImportsCheck, which the project already uses twice, and
   runs in the existing checkstyleTest job. There is no new tooling or
   workflow required.
   3.

   *Corrections to the existing integration testing chapter:* While writing
   the new one, I found that the current docs outline four base class methods
   that do not exist, three Gradle options the test task does not accept, and
   it still teaches the REST Assured setup block as the way to start a new
   test. It also does not mention -PcargoDisabled, which is required to run
   against your own instance.

*Note: There are no production or test code changes in this PR.*
Please feel free to check it out and look into

   -

   *Rule accuracy:* Are the rules in the new chapter the right ones? I have
   deliberately kept them to points that came out of actual review feedback,
   but I may have included something that is only my own preference or missed
   something that matters more.
   -

   *Formatting:* Is the numbered rule format (IT-01, IT-02, etc.) useful,
   or is it more structure than this needs?
   -

   *Checkstyle scope:* The Checkstyle rule covers the shared base classes
   and helpers, not the migrated tests themselves. Most of those live in the
   root integrationtests package alongside the tests that still use REST
   Assured, and Checkstyle can only scope by package, not by parent class. I
   would rather guard what is realistic today than exclude files to make a
   wider rule pass, but I am open to other suggestions.

*One note on ordering:* The Checkstyle rule reports five violations against
develop today, all in FeignLoanHelper. Those are removed by #6321 [3],
which is already open. This pull request should go in after that one is
merged.

Comments on the pull request are welcome and probably easier to follow than
the mailing list for the detailed points.

Thanks, Krishna

[1] https://issues.apache.org/jira/browse/FINERACT-2781
[2] https://github.com/apache/fineract/pull/6324
[3] https://github.com/apache/fineract/pull/6321
[4] https://issues.apache.org/jira/browse/FINERACT-2609

Reply via email to