Skip to content

Conversation

@RyanM-RMA
Copy link
Collaborator

Related to #1349, I'll be adding a unit test for static analysis of of the Controller implementations.

@RyanM-RMA RyanM-RMA force-pushed the bugfix/CDA-61_controller_param_analysis branch from d2fac51 to 96abc89 Compare October 29, 2025 17:49
… a specific package that inherits from a provided type.
…DocInfo for improved parameter handling, and add tests for interface-based handlers.
… introduce OpenApiDocTestInfo, and update tests accordingly. Add new OpenApiDocTest for handler and CRUD handler documentation validation.
…-based static analysis for improved validation and modularized helper methods.
…oduce OpenApiParamInfo for enhanced parameter metadata handling. Introducing a quick test for TimeSeriesController for faster turnaround.
…out consistency with how the ignored methods are handled.
…or related methods for improved query and path parameter parsing.
…ist` for parameter collections so it includes order, add null-handling tracking, and enhance test coverage for query and path parameter validation.
…arameter issues and using set instead of list for parameter usage
…amInfo` with constants from `Controllers`.
@RyanM-RMA RyanM-RMA marked this pull request as ready for review November 12, 2025 02:41
@rma-rripken
Copy link
Collaborator

This ended up needing more code from JavaParser than I anticipated from the beginning. I don't know JavaParser well enough to know if there are shortcuts we are missing but from what I've seen in the results it is catching the errors I was hoping we'd find.

Clearing up messaging for documented unused parameters vs undocumented used parameters.
Adding code for resolving references and fields and such down to their actual values.
rma-rripken
rma-rripken previously approved these changes Nov 24, 2025
Copy link
Collaborator

@rma-rripken rma-rripken left a comment

Choose a reason for hiding this comment

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

We should check with Mike on whether we submit this enabled or not? Maybe a @tag on the test class so that its submitted but not run during the automated build?

@MikeNeilson
Copy link
Contributor

We should check with Mike on whether we submit this enabled or not? Maybe a @tag on the test class so that its submitted but not run during the automated build?

Gentleman, I shall scream it again... EnableIfProperty! and then add another job to the build so we see the results but it doesn't block the builds yet.

@RyanM-RMA
Copy link
Collaborator Author

We should check with Mike on whether we submit this enabled or not? Maybe a @tag on the test class so that its submitted but not run during the automated build?

Gentleman, I shall scream it again... EnableIfProperty! and then add another job to the build so we see the results but it doesn't block the builds yet.

Oooh, I don't believe I have used this, thank you for the quick feedback @MikeNeilson. Give me a few minutes, and I'll get that going.

@MikeNeilson
Copy link
Contributor

We should check with Mike on whether we submit this enabled or not? Maybe a @tag on the test class so that its submitted but not run during the automated build?

Gentleman, I shall scream it again... EnableIfProperty! and then add another job to the build so we see the results but it doesn't block the builds yet.

Oooh, I don't believe I have used this, thank you for the quick feedback @MikeNeilson. Give me a few minutes, and I'll get that going.

FYI did the name for memory, it might be slightly different than that exact text.

…a new job to run the static analysis separately.
- name: build and test
id: thebuild
run: ./gradlew :cwms-data-api:test --tests "cwms.cda.api.OpenApiDocTest" --info --init-script init.gradle
continue-on-error: true
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this and set the below to "always"

We've tweaked the repo to have specific "required builds" this one for now is informational, but we still want it to show up as "failed" to draw eyes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I updated this right, let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, though looks like you need to update the branch, below is still showing the update button.

@RyanM-RMA RyanM-RMA merged commit 4f58e8f into USACE:develop Nov 24, 2025
6 of 7 checks passed
@RyanM-RMA RyanM-RMA deleted the bugfix/CDA-61_controller_param_analysis branch November 24, 2025 23:14
vairav added a commit that referenced this pull request Dec 8, 2025
…ilters

* origin:
  Entity endpoint Controller and Integration test (#1497)
  Enhancements/blob clob query (#1483)
  Update treafik for latest docker. (#1493)
  Bugfix/cda 45 ts vertical datum (#1344)
  CDA-66: Updated TS identifier descriptor paging (#1481)
  add in missing expiration date to constant/seasonal levels (#1490)
  1351 implement cda gui code formatter (#1460)
  add missing back tic
  CWMS Data API documentation /timeseries GET endpoints. (#1476)
  CDA-60: Accept Header Formatting Documentation (#1463)
  The temp users set needs to be a LinkedHashSet, otherwise the last user in the list isn't always the last user and the paging doesn't work.
  Add static analysis unit test for Controller classes (#1362)
  Bugfix/incorrect parameter warning cda 58 (#1470)
  Test updates for latest schema and correct release schema image. (#1474)
  CDA-54 - Implements Entity DTO and Dao (#1482)
  Update npm pacakges (#1478)
  Correct required java version (#1462)
  CDA-40: Exception Handling Implementation Updates (#1358)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create unit test that does static analysis of ALL Controllers and finds parameter mismatches

3 participants