-
Notifications
You must be signed in to change notification settings - Fork 108
fix: [#749] The path is incorrect when publishing package files #1157
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request fixes incorrect path handling in the vendor publish command by removing redundant path normalization logic and adds test coverage for error scenarios.
- Removed unnecessary path trimming logic that was causing incorrect paths during package file publishing
- Added comprehensive test coverage for error handling scenarios using mock contexts
- Cleaned up unused imports and minor formatting improvements
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| foundation/console/vendor_publish_command.go | Removed redundant path normalization and unused strings import |
| foundation/console/vendor_publish_command_test.go | Added new test cases for error handling scenarios with mock framework |
| foundation/console/package_make_command_stubs.go | Minor formatting cleanup removing extra blank line |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make more tests in goravel/example
| } | ||
|
|
||
| for sourcePath, targetValue := range paths { | ||
| targetValue = strings.TrimPrefix(strings.TrimPrefix(targetValue, "/"), "./") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use the original path.
almas-x
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* correctly set cc and bcc headers (#1144) * correct the error return from SendMailJob handle (#1147) * fix: [#743] package make command generates correct code (#1151) (#1152) (cherry picked from commit 93dc8a3) * fix: [#749] The path is incorrect when publishing package files (#1157) * chore: optimize assertions for package installation (#1160) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting (#1162) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting * optimize * fix: [#738] The Orm Creating event can be triggered when the query with the Model method (#1166) * fix: [#738] The Orm Creating event can be triggered when the query with the Model method * fix tests * upgrade: v1.16.1 * fix: [#762] handle panic when using transaction (#1183) * fix: [#762] handle panic when using transaction * v1.16.2 * optimize * fix lint * fix: [#768] facades.DB will panic when migrating a new column (#1185) * fix: [#768] facades.DB will panic when migrating a new column * optimize * optimize * feat: [#770] Add a SelectRaw function for the ORM (#1186) * fix: [#770] Add a SelectRaw function for the ORM * fix: [#770] Add a SelectRaw function for the ORM * fix ci * upgrade: v1.16.3 * fix typo --------- Co-authored-by: krishan kumar <[email protected]> Co-authored-by: ALMAS <[email protected]>
* correctly set cc and bcc headers (#1144) * correct the error return from SendMailJob handle (#1147) * fix: [#743] package make command generates correct code (#1151) (#1152) (cherry picked from commit 93dc8a3) * fix: [#749] The path is incorrect when publishing package files (#1157) * chore: optimize assertions for package installation (#1160) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting (#1162) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting * optimize * fix: [#738] The Orm Creating event can be triggered when the query with the Model method (#1166) * fix: [#738] The Orm Creating event can be triggered when the query with the Model method * fix tests * upgrade: v1.16.1 * fix: [#762] handle panic when using transaction (#1183) * fix: [#762] handle panic when using transaction * v1.16.2 * optimize * fix lint * fix: [#768] facades.DB will panic when migrating a new column (#1185) * fix: [#768] facades.DB will panic when migrating a new column * optimize * optimize * feat: [#770] Add a SelectRaw function for the ORM (#1186) * fix: [#770] Add a SelectRaw function for the ORM * fix: [#770] Add a SelectRaw function for the ORM * fix ci * upgrade: v1.16.3 * fix: comand cannot be run concurrently (#1243) * fix: comand cannot be run concurrently * fix ci * fix ci * optimize * optimize * optimize * optimize * optimize global options * fix ci * upgrade v1.16.4 * optimize * optimize --------- Co-authored-by: krishan kumar <[email protected]> Co-authored-by: ALMAS <[email protected]>
* correctly set cc and bcc headers (#1144) * correct the error return from SendMailJob handle (#1147) * fix: [#743] package make command generates correct code (#1151) (#1152) (cherry picked from commit 93dc8a3) * fix: [#749] The path is incorrect when publishing package files (#1157) * chore: optimize assertions for package installation (#1160) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting (#1162) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting * optimize * fix: [#738] The Orm Creating event can be triggered when the query with the Model method (#1166) * fix: [#738] The Orm Creating event can be triggered when the query with the Model method * fix tests * upgrade: v1.16.1 * fix: [#762] handle panic when using transaction (#1183) * fix: [#762] handle panic when using transaction * v1.16.2 * optimize * fix lint * fix: [#768] facades.DB will panic when migrating a new column (#1185) * fix: [#768] facades.DB will panic when migrating a new column * optimize * optimize * feat: [#770] Add a SelectRaw function for the ORM (#1186) * fix: [#770] Add a SelectRaw function for the ORM * fix: [#770] Add a SelectRaw function for the ORM * fix ci * upgrade: v1.16.3 * fix: comand cannot be run concurrently (#1243) * fix: comand cannot be run concurrently * fix ci * fix ci * optimize * optimize * optimize * optimize * optimize global options * fix ci * upgrade v1.16.4 * fix: [#807] queue.Shutdown doesn't stop the queue as expected (#1252) * fix: [#807] queue.Shutdown doesn't stop the queue as expected * optimize * upgrade: v1.16.5 * fix * Update queue/worker_test.go Co-authored-by: Copilot <[email protected]> * optimize --------- Co-authored-by: krishan kumar <[email protected]> Co-authored-by: ALMAS <[email protected]> Co-authored-by: Copilot <[email protected]>
* correctly set cc and bcc headers (#1144) * correct the error return from SendMailJob handle (#1147) * fix: [#743] package make command generates correct code (#1151) (#1152) (cherry picked from commit 93dc8a3) * fix: [#749] The path is incorrect when publishing package files (#1157) * chore: optimize assertions for package installation (#1160) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting (#1162) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting * optimize * fix: [#738] The Orm Creating event can be triggered when the query with the Model method (#1166) * fix: [#738] The Orm Creating event can be triggered when the query with the Model method * fix tests * upgrade: v1.16.1 * fix: [#762] handle panic when using transaction (#1183) * fix: [#762] handle panic when using transaction * v1.16.2 * optimize * fix lint * fix: [#768] facades.DB will panic when migrating a new column (#1185) * fix: [#768] facades.DB will panic when migrating a new column * optimize * optimize * feat: [#770] Add a SelectRaw function for the ORM (#1186) * fix: [#770] Add a SelectRaw function for the ORM * fix: [#770] Add a SelectRaw function for the ORM * fix ci * upgrade: v1.16.3 * fix: comand cannot be run concurrently (#1243) * fix: comand cannot be run concurrently * fix ci * fix ci * optimize * optimize * optimize * optimize * optimize global options * fix ci * upgrade v1.16.4 * fix: [#807] queue.Shutdown doesn't stop the queue as expected (#1252) * fix: [#807] queue.Shutdown doesn't stop the queue as expected * optimize * upgrade: v1.16.5 * fix * Update queue/worker_test.go Co-authored-by: Copilot <[email protected]> * optimize --------- Co-authored-by: krishan kumar <[email protected]> Co-authored-by: ALMAS <[email protected]> Co-authored-by: Copilot <[email protected]>
📑 Description
Closes goravel/goravel#749
This pull request introduces improvements to the vendor publish command and its test coverage, along with minor code cleanups. The main changes include refining path handling logic, enhancing test coverage for error scenarios, and cleaning up unused imports.
Vendor publish command improvements:
Handlemethod invendor_publish_command.go, simplifying how target paths are processed.Test coverage enhancements:
vendor_publish_command_test.goto verify error handling when no vendor is found and when the package directory does not exist. These tests use a mock context to simulate command execution and error reporting.github.com/stretchr/testify/mockand a new mock context (mocksconsole) to facilitate more robust and isolated testing of command behavior.Code cleanup:
stringsimport fromvendor_publish_command.go, reflecting the simplification of path handling logic.package_make_command_stubs.gofor improved readability.✅ Checks