Service: Add location tests for views #2496
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive test coverage for location validation of Iceberg views, ensuring that views respect allowed location constraints similar to tables.
- Adds two new test methods that verify view creation and updates within and outside allowed locations
- Introduces supporting test infrastructure including view constants and helper methods
- Imports necessary dependencies for view testing functionality
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| createCatalog(services, Map.of(), catalogBaseLocation, List.of(catalogBaseLocation)); | ||
| createNamespace(services, namespaceLocation); | ||
|
|
||
| var locationNotAllowed = Paths.get(tmpDir.toUri().toString()).toString(); |
There was a problem hiding this comment.
Converting URI to string and then back to Path is unnecessary. Use tmpDir.toAbsolutePath().toString() directly for consistency with other location variables in the test.
| var locationNotAllowed = Paths.get(tmpDir.toUri().toString()).toString(); | |
| var locationNotAllowed = tmpDir.toAbsolutePath().toString(); |
There was a problem hiding this comment.
this comment makes sense to me... however, the code suggestion probably needs to be tmpDir.toAbsolutePath().toUri().toString() to match line 208.
There was a problem hiding this comment.
Using this to make it more explicitly
tmpDir.resolve("location-not-allowed").toAbsolutePath().toUri().toString();
flyrain
changed the title
|
The test failure isn't related It seems a flaky test introduced by #1844. cc @adnanhemani |
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
github-project-automation
bot
moved this from Ready to merge
to Done
in Basic Kanban Board
2 participants
No description provided.