search

apache / gravitino

World's most powerful open data catalog for building a high-performance, geo-distributed and federated metadata lake.
https://gravitino.apache.org
Apache License 2.0
1.09k stars 343 forks source link

[#5637] Add CLI unit tests to check if the right command has been called. #5638

Open justinmclean opened 1 day ago

justinmclean commented 1 day ago

What changes were proposed in this pull request?

Add CLI unit tests to check if the right command has been called.

Why are the changes needed?

To increate unit test coverage.

Fix: #5637

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

Tested locally.

justinmclean commented 1 day ago

Not complete yet, I've just added the tests for Metalakes. @jerryshao or @shaofengshi can you confirm this is the right direction before I create tests for other entities.

jerryshao commented 19 hours ago

Looks like you only added the metalake related tests, will you add more tests about others? Also, I saw that your test cases are more about good tests, can you please also add some bad tests?

justinmclean commented 19 hours ago

Yes, I will add more tests, as I said above, "can you confirm this is the right direction before I create tests for other entities."

justinmclean commented 18 hours ago

With the metalakes commands, there are no bad paths in this path code. Missing a --value flag from a property command is handled by the CLI library. Same with a missing a property flag in metalake remove command. Same with an unknown flag. Same with not passing a value to a command like --rename. Unknown entities and commands already have unit tests. The above generic conditions are also covered by unit tests in TestMain.java.

jerryshao commented 16 hours ago

Yeah, I'm fine with the current direction, please add more tests to cover all the options/flags as possible as we can.

justinmclean commented 22 minutes ago

@jerryshao / @shaofengshi, this is ready for review.