Skip to content

feat: add name option to customize the encrypted environment file name #911

Merged
hwbrzzl merged 11 commits intogoravel:masterfrom
go-projects:master
Feb 23, 2025
Merged

feat: add name option to customize the encrypted environment file name #911
hwbrzzl merged 11 commits intogoravel:masterfrom
go-projects:master

Conversation

@kuafuRace
Copy link
Contributor

@kuafuRace kuafuRace commented Feb 23, 2025

📑 Description

  • add name option to customize the encrypted environment file name, such as
go run .artisan env:encrypt --name .env.safe
go run .artisan env:decrypt --name .env.safe --key BgcELROHL8sAV568T7Fiki7krjLHOkUc

If no name is specified, the default encrypted environment file name is .env.encrypted

  • improve unit testing coverage for package_make_command.go, test_make_command.go, vendor_publish_command.go, etc
  • replace .env with support.EnvPath
  • replace .env.encrypted with support.EnvEncryptPath
  • unified unit test format

Summary by CodeRabbit

  • New Features

    • Added a new option in the encryption and decryption commands that lets users specify custom names for encrypted environment files.
    • Introduced new test suites for various command functionalities, enhancing test organization and coverage.
  • Bug Fixes

    • Removed specific error messages related to AES encryption to simplify error handling.
  • Refactor

    • Streamlined encryption error handling by removing redundant validations, resulting in clearer feedback when issues occur.
    • Updated test methods to utilize consistent naming conventions and improved clarity in test structure.
    • Enhanced organization of test cases by adopting a suite structure for better readability and maintainability.
    • Added new constants for environment encryption paths and cipher types to improve maintainability.

✅ Checks

  • Added test cases for my code

@kuafuRace kuafuRace requested a review from a team as a code owner February 23, 2025 04:12
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2025

Walkthrough

The changes include the removal of two AES-related error constants, modifications to command handling for environment encryption and decryption, and extensive test refactoring. New command-line flags have been added to allow custom file naming while updating file path usage to reference support constants. Several test suites have been restructured using the testify suite pattern with improved variable naming and clarity. Additionally, two new support constants have been introduced and existing ones initialized to default values.

Changes

File(s) Change Summary
errors/list.go Removed error variables AesCiphertextInvalid and AesPaddingInvalid related to AES encryption validation.
foundation/console/env_decrypt_command.go
foundation/console/env_encrypt_command.go
Introduced new name flag to customize encrypted file names; updated file operations to use support.EnvPath and support.EnvEncryptPath; simplified error handling in decryption.
foundation/console/about_command_test.go
foundation/console/env_decrypt_command_test.go
foundation/console/env_encrypt_command_test.go
foundation/console/package_make_command_test.go
foundation/console/test_make_command_test.go
foundation/console/vendor_publish_command_test.go
Refactored tests to adopt suite-based structures; renamed context and command variables (e.g., from mockContext to ctx and from aboutCommand to cmd); replaced hardcoded file paths with support constants; added new test methods for command signatures, descriptions, and extension properties.
support/constant.go Added new variables EnvEncryptPath = ".env.encrypted" and EnvEncryptCipher = "AES-256-CBC"; initialized RelativePath and RootPath to empty strings.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Command
    participant FS as "File System"
    participant Support

    User->>Command: Invoke encrypt/decrypt with --name flag
    Command->>Support: Retrieve EnvPath and EnvEncryptPath
    Command->>FS: Read/Write file using provided flag value
    FS-->>Command: Return file content/result
    Command->>User: Output result message with cipher info and file name
Loading

Suggested Reviewers

  • hwbrzzl
  • devhaozi

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 golangci-lint (1.62.2)

Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0)
Failed executing command with error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0)


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ef6c75 and 2782a3e.

📒 Files selected for processing (2)
  • foundation/console/env_decrypt_command_test.go (3 hunks)
  • foundation/console/env_encrypt_command_test.go (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • foundation/console/env_decrypt_command_test.go
  • foundation/console/env_encrypt_command_test.go
⏰ Context from checks skipped due to timeout of 300000ms (1)
  • GitHub Check: test / windows (1.23)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@kuafuRace kuafuRace changed the title feat: add 'name' option to customize the encrypted environment file name feat: add name option to customize the encrypted environment file name Feb 23, 2025
@codecov
Copy link

codecov bot commented Feb 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.86%. Comparing base (06bd44b) to head (2782a3e).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #911      +/-   ##
==========================================
+ Coverage   68.01%   68.86%   +0.84%     
==========================================
  Files         154      154              
  Lines       10192    10196       +4     
==========================================
+ Hits         6932     7021      +89     
+ Misses       2934     2852      -82     
+ Partials      326      323       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kuafuRace
Copy link
Contributor Author

Ready Review

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
foundation/console/env_encrypt_command.go (1)

104-104: ⚠️ Potential issue

Security concern: Predictable IV generation.

Using the key as the IV is not recommended as it makes the encryption more predictable. Consider using a cryptographically secure random IV.

-	iv := key[:aes.BlockSize]
+	iv := make([]byte, aes.BlockSize)
+	if _, err := rand.Read(iv); err != nil {
+		return nil, fmt.Errorf("failed to generate IV: %w", err)
+	}
🧹 Nitpick comments (5)
foundation/console/env_decrypt_command.go (2)

81-85: Enhance error handling with specific error types.

The current error handling could be more informative by providing specific error types for different failure scenarios (e.g., decryption failures vs. file system errors).

 	plaintext, err := r.decrypt(ciphertext, []byte(key))
 	if err != nil {
-		ctx.Error(fmt.Sprintf("Decrypt error: %v", err))
+		switch {
+		case errors.Is(err, base64.CorruptInputError(0)):
+			ctx.Error("Invalid base64 encoding in encrypted file")
+		case errors.Is(err, aes.KeySizeError(0)):
+			ctx.Error("Invalid key size for decryption")
+		default:
+			ctx.Error(fmt.Sprintf("Decrypt error: %v", err))
+		}
 		return nil
 	}

87-87: Consider using a constant for file permissions.

The file permission 0644 is hardcoded. Consider defining it as a constant for better maintainability and reusability.

+const envFilePermissions = 0644

-	err = os.WriteFile(support.EnvPath, plaintext, 0644)
+	err = os.WriteFile(support.EnvPath, plaintext, envFilePermissions)
foundation/console/test_make_command_test.go (1)

71-100: Consider adding cleanup in a TearDown method.

The file cleanup at the end of TestTestHandle could be moved to a suite-level TearDown method to ensure cleanup happens even if tests fail.

+func (s *TestMakeCommandTestSuite) TearDownTest() {
+	if file.Exists("tests") {
+		s.NoError(file.Remove("tests"))
+	}
+}

 func (s *TestMakeCommandTestSuite) TestTestHandle() {
     // ... existing test code ...
-    s.NoError(file.Remove("tests"))
 }
foundation/console/vendor_publish_command_test.go (2)

39-72: Consider simplifying the TestExtend method.

While the test is thorough, it could be simplified:

  1. The type assertion error message uses an incorrect format string (%T without arguments).
  2. The deep equality comparison could be simplified by directly using s.Equal instead of manual reflection.

Consider this simplified version:

 func (s *VendorPublishCommandTestSuite) TestExtend() {
 	cmd := &VendorPublishCommand{}
 	got := cmd.Extend()
 
 	s.Run("should return correct category", func() {
 		expected := "vendor"
 		s.Require().Equal(expected, got.Category)
 	})
 
 	if len(got.Flags) > 0 {
 		s.Run("should have correctly configured StringFlag", func() {
 			flag, ok := got.Flags[0].(*command.BoolFlag)
-			if !ok {
-				s.Fail("existing flag is not BoolFlag (got type: %T)", got.Flags[0])
-			}
+			s.Require().True(ok, "existing flag is not BoolFlag (got type: %T)", got.Flags[0])
 
-			testCases := []struct {
-				name     string
-				got      interface{}
-				expected interface{}
-			}{
-				{"Name", flag.Name, "existing"},
-				{"Aliases", flag.Aliases, []string{"e"}},
-				{"Usage", flag.Usage, "Publish and overwrite only the files that have already been published"},
-			}
-
-			for _, tc := range testCases {
-				if !reflect.DeepEqual(tc.got, tc.expected) {
-					s.Require().Equal(tc.expected, tc.got)
-				}
-			}
+			s.Equal("existing", flag.Name, "flag name should match")
+			s.Equal([]string{"e"}, flag.Aliases, "flag aliases should match")
+			s.Equal("Publish and overwrite only the files that have already been published", flag.Usage, "flag usage should match")
 		})
 	}
 }

310-424: Improve error handling and cleanup in file operation tests.

While the tests are comprehensive, consider these improvements:

  1. Use t.TempDir() instead of os.MkdirTemp() for automatic cleanup
  2. Replace s.Nil(err) with more descriptive s.NoError(err)
  3. Use defer for cleanup immediately after resource creation

Example improvement for the TestPublish method:

 func (s *VendorPublishCommandTestSuite) TestPublish() {
 	cmd := &VendorPublishCommand{}
 
-	sourceDir, err := os.MkdirTemp("", "source")
-	s.Require().Nil(err)
-	defer func(path string) {
-		if err := file.Remove(path); err != nil {
-			panic(err)
-		}
-	}(sourceDir)
+	sourceDir := s.T().TempDir()
+	targetDir := s.T().TempDir()
 
-	targetDir, err := os.MkdirTemp("", "target")
-	s.Require().Nil(err)
-
 	sourceFile := filepath.Join(sourceDir, "test.txt")
-	s.Require().Nil(file.PutContent(sourceFile, "test"))
+	s.Require().NoError(file.PutContent(sourceFile, "test"), "Failed to create test file")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 06bd44b and ef6d137.

📒 Files selected for processing (10)
  • errors/list.go (0 hunks)
  • foundation/console/about_command_test.go (3 hunks)
  • foundation/console/env_decrypt_command.go (3 hunks)
  • foundation/console/env_decrypt_command_test.go (2 hunks)
  • foundation/console/env_encrypt_command.go (3 hunks)
  • foundation/console/env_encrypt_command_test.go (2 hunks)
  • foundation/console/package_make_command_test.go (4 hunks)
  • foundation/console/test_make_command_test.go (1 hunks)
  • foundation/console/vendor_publish_command_test.go (13 hunks)
  • support/constant.go (1 hunks)
💤 Files with no reviewable changes (1)
  • errors/list.go
🔇 Additional comments (21)
support/constant.go (2)

14-15: LGTM! Well-defined constants for environment encryption.

The new constants follow a consistent naming convention and provide clear defaults for the encryption feature.


17-18: Good practice: Initializing path variables with empty strings.

This change prevents potential nil pointer dereferences and makes the behavior more predictable.

foundation/console/env_decrypt_command.go (1)

45-50: LGTM! Well-structured flag definition.

The new flag follows the consistent pattern with other flags and provides clear usage information.

foundation/console/env_encrypt_command.go (2)

47-52: LGTM! Consistent flag definition with decrypt command.

The flag definition matches the decrypt command, maintaining symmetry between the two commands.


84-84: Consider using a constant for file permissions.

Similar to the decrypt command, the file permission 0644 should be defined as a constant.

foundation/console/test_make_command_test.go (2)

16-22: LGTM! Well-structured test suite setup.

The conversion to a testify suite improves test organization and follows the project's testing patterns.


36-69: LGTM! Comprehensive flag testing.

The test cases thoroughly verify the flag configuration using table-driven tests and proper type assertions.

foundation/console/package_make_command_test.go (3)

16-22: LGTM! Well-structured test suite implementation.

The test suite is properly structured using testify's suite pattern, which improves test organization and maintainability.


34-68: LGTM! Comprehensive test coverage for command flags.

The test implementation thoroughly validates the command's flags using structured test cases and proper type assertions. The use of reflect.DeepEqual for comparisons is a good practice.


70-113: LGTM! Well-organized test cases with proper cleanup.

The test cases are well-structured with clear setup and assertions. The cleanup of test files is properly handled using deferred functions.

foundation/console/about_command_test.go (3)

20-29: LGTM! Clean test suite implementation.

The test suite is properly structured and follows the same pattern as other command tests.


31-41: LGTM! Clear and focused test methods.

The test methods are well-focused and validate specific aspects of the command (signature and description).


43-70: LGTM! Thorough flag validation.

The test implementation properly validates the command's flags using structured test cases and type assertions.

foundation/console/env_decrypt_command_test.go (3)

81-90: LGTM! Proper implementation of the new name option.

The test cases correctly validate the new name option functionality while maintaining existing behavior.


92-103: LGTM! Good error handling test coverage.

The test cases properly validate error scenarios with the new option.


173-185: LGTM! Thorough encryption validation.

The test cases properly validate both successful and failed encryption scenarios.

foundation/console/env_encrypt_command_test.go (3)

76-86: LGTM! Consistent implementation with decrypt command.

The test cases for the encrypt command mirror those of the decrypt command, ensuring consistent behavior.


120-144: LGTM! Comprehensive success scenario testing.

The test cases thoroughly validate successful encryption scenarios with proper file handling and cleanup.


167-179: LGTM! Good encryption validation.

The test cases properly validate both successful and failed encryption operations.

foundation/console/vendor_publish_command_test.go (2)

27-37: LGTM! Well-structured basic tests.

The TestSignature and TestDescription methods are well-implemented with clear assertions and good error messages.


75-76: LGTM! Consistent variable naming.

The renaming of command to cmd across all test methods improves consistency and follows Go's idiomatic practices.

Also applies to: 105-106, 221-222, 304-305, 311-312, 382-383

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Thanks, great PR, just some nitpicks.

mockConfig := mocksconfig.NewConfig(t)

cmd := NewAboutCommand(mockApp)
ctx := &mocksconsole.Context{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ctx := &mocksconsole.Context{}
mockContext := mocksconsole.NewContext(t)

}
}

func TestAboutCommand(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge this function to the suite.

}

if file.Exists(".env") {
if file.Exists(support.EnvPath) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch

envDecryptCommand := NewEnvDecryptCommand()
mockContext := mocksconsole.NewContext(s.T())
cmd := NewEnvDecryptCommand()
ctx := mocksconsole.NewContext(s.T())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ctx := mocksconsole.NewContext(s.T())
mockContext := mocksconsole.NewContext(s.T())

envEncryptCommand := NewEnvEncryptCommand()
mockContext := mocksconsole.NewContext(s.T())
cmd := NewEnvEncryptCommand()
ctx := mocksconsole.NewContext(s.T())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ctx := mocksconsole.NewContext(s.T())
mockContext := mocksconsole.NewContext(s.T())

assert.True(t, file.Contain("packages/sms/facades/sms.go", "goravel/packages/sms"))
assert.True(t, file.Contain("packages/sms/facades/sms.go", "goravel/packages/sms/contracts"))
assert.NoError(t, file.Remove("packages"))
s.NoError(NewPackageMakeCommand().Handle(mockContext))
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch


func (s *TestMakeCommandTestSuite) TestTestHandle() {
cmd := NewTestMakeCommand()
ctx := mocksconsole.NewContext(s.T())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ctx := mocksconsole.NewContext(s.T())
mockContext := mocksconsole.NewContext(s.T())

@hwbrzzl
Copy link
Contributor

hwbrzzl commented Feb 23, 2025

And about the Test External CI, I'm changing some logic, it's expected to fail.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (1)
foundation/console/about_command_test.go (1)

71-139: ⚠️ Potential issue

Fix test expectations to match method calls.

The pipeline failures indicate that the test expectations for TwoColumnDetail and NewLine methods don't match the actual calls.

The test is failing because:

  1. The order of sections in the test output doesn't match the expectations
  2. The number of NewLine calls might be incorrect

Verify and fix the test expectations:

  1. Ensure the sections are added in the correct order
  2. Verify the number of NewLine calls
  3. Double-check the content of each TwoColumnDetail call
🧰 Tools
🪛 GitHub Actions: Codecov

[error] 138-138: TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'.


[error] 99-99: NewLine() method failed.


[error] 130-130: TwoColumnDetail(string,string) method failed.


[error] 137-137: TwoColumnDetail(string,string) method failed.

🧹 Nitpick comments (3)
foundation/console/test_make_command_test.go (2)

36-69: Improve test efficiency and error handling.

Consider the following improvements:

  1. The type assertion error message uses incorrect formatting.
  2. The DeepEqual check followed by Equal assertion is redundant.

Apply this diff to improve the code:

-				s.Fail("First flag is not BoolFlag (got type: %T)", got.Flags[0])
+				s.Failf("First flag is not BoolFlag", "got type: %T", got.Flags[0])

-			for _, tc := range testCases {
-				if !reflect.DeepEqual(tc.got, tc.expected) {
-					s.Require().Equal(tc.expected, tc.got)
-				}
-			}
+			for _, tc := range testCases {
+				s.Require().Equal(tc.expected, tc.got, tc.name)
+			}

75-100: Consider using subtests for better organization.

While the test cases are comprehensive, they could be more readable if organized into subtests using s.Run(). This would make it easier to identify which scenario failed and provide better test output.

Example structure:

func (s *TestMakeCommandTestSuite) TestTestHandle() {
	cmd := NewTestMakeCommand()
	
	s.Run("should handle empty test name", func() {
		mockContext := mocksconsole.NewContext(s.T())
		mockContext.EXPECT().Argument(0).Return("").Once()
		mockContext.EXPECT().Ask("Enter the test name", mock.Anything).Return("", errors.New("the test name cannot be empty")).Once()
		mockContext.EXPECT().Error("the test name cannot be empty").Once()
		s.Nil(cmd.Handle(mockContext))
	})
	
	s.Run("should create test successfully", func() {
		mockContext := mocksconsole.NewContext(s.T())
		mockContext.EXPECT().Argument(0).Return("UserTest").Once()
		mockContext.EXPECT().OptionBool("force").Return(false).Once()
		mockContext.EXPECT().Success("Test created successfully").Once()
		s.NoError(cmd.Handle(mockContext))
		s.True(file.Exists("tests/user_test.go"))
	})
	
	// ... more subtests ...
	
	s.NoError(file.Remove("tests"))
}
foundation/console/about_command_test.go (1)

42-69: Simplify flag testing using testify's assertions.

The flag testing logic using reflection can be simplified using testify's built-in assertions.

-			testCases := []struct {
-				name     string
-				got      interface{}
-				expected interface{}
-			}{
-				{"Name", flag.Name, "only"},
-				{"Usage", flag.Usage, "The section to display"},
-			}
-
-			for _, tc := range testCases {
-				if !reflect.DeepEqual(tc.got, tc.expected) {
-					s.Require().Equal(tc.expected, tc.got)
-				}
-			}
+			s.Equal("only", flag.Name, "Flag name should be 'only'")
+			s.Equal("The section to display", flag.Usage, "Flag usage should match expected")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef6d137 and e79975e.

📒 Files selected for processing (4)
  • foundation/console/about_command_test.go (3 hunks)
  • foundation/console/env_decrypt_command_test.go (2 hunks)
  • foundation/console/env_encrypt_command_test.go (2 hunks)
  • foundation/console/test_make_command_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • foundation/console/env_decrypt_command_test.go
  • foundation/console/env_encrypt_command_test.go
🧰 Additional context used
🪛 GitHub Actions: Codecov
foundation/console/about_command_test.go

[error] 138-138: TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'.


[error] 99-99: NewLine() method failed.


[error] 130-130: TwoColumnDetail(string,string) method failed.


[error] 137-137: TwoColumnDetail(string,string) method failed.

⏰ Context from checks skipped due to timeout of 300000ms (2)
  • GitHub Check: test / windows (1.24)
  • GitHub Check: test / windows (1.23)
🔇 Additional comments (5)
foundation/console/test_make_command_test.go (4)

3-22: LGTM! Well-structured test suite setup.

The imports are well-organized, and the test suite is properly structured using testify's suite pattern.


24-28: LGTM! Clear and focused test.

The test follows the Arrange-Act-Assert pattern and uses appropriate assertions.


30-34: LGTM! Well-structured test.

The test follows the Arrange-Act-Assert pattern and uses appropriate assertions.


71-74: LGTM! Correct mock context setup.

The mock context creation follows the suggested pattern from past reviews.

foundation/console/about_command_test.go (1)

19-24: LGTM! Test suite setup follows best practices.

The test suite structure is well-organized using testify's suite package. Consider adding common setup logic in the SetupTest method if needed in the future.

Comment on lines 136 to 139
AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})
assert.Contains(t, color.CaptureOutput(func(w io.Writer) {
assert.Nil(t, aboutCommand.Handle(mockContext))
s.Contains(color.CaptureOutput(func(w io.Writer) {
s.Nil(cmd.Handle(mockContext))
}), strings.Join(expected, ""))
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Address pipeline failures in test assertions.

The test is failing due to mismatched expectations. The pipeline failures indicate:

TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'

Fix the test by:

  1. Moving the AddAboutInformation call before setting up the expectations
  2. Including the custom section in the expectations array
+	AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})
 	var expected []string
 	for _, ex := range [][2]string{
 		{"<fg=green;op=bold>Environment</>", ""},
		// ... other sections ...
 	} {
 		mockContext.EXPECT().TwoColumnDetail(ex[0], ex[1]).
 			Run(func(first string, second string, _ ...rune) {
 				expected = append(expected, color.Default().Sprintf("%s %s\n", first, second))
 				color.Default().Printf("%s %s\n", first, second)
 			}).Return().Once()
 	}
-	AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})
assert.Contains(t, color.CaptureOutput(func(w io.Writer) {
assert.Nil(t, aboutCommand.Handle(mockContext))
s.Contains(color.CaptureOutput(func(w io.Writer) {
s.Nil(cmd.Handle(mockContext))
}), strings.Join(expected, ""))
// Move the AddAboutInformation call before setting up expectations
AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})
var expected []string
for _, ex := range [][2]string{
{"<fg=green;op=bold>Environment</>", ""},
// ... other sections, including expected custom section if applicable ...
} {
mockContext.EXPECT().TwoColumnDetail(ex[0], ex[1]).
Run(func(first string, second string, _ ...rune) {
expected = append(expected, color.Default().Sprintf("%s %s\n", first, second))
color.Default().Printf("%s %s\n", first, second)
}).Return().Once()
}
s.Contains(color.CaptureOutput(func(w io.Writer) {
s.Nil(cmd.Handle(mockContext))
}), strings.Join(expected, ""))
// Removed duplicate AddAboutInformation call here
🧰 Tools
🪛 GitHub Actions: Codecov

[error] 138-138: TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'.


[error] 137-137: TwoColumnDetail(string,string) method failed.

…ColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'
Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Great PR, thanks!

@hwbrzzl hwbrzzl merged commit 96a1644 into goravel:master Feb 23, 2025
12 of 14 checks passed
almas-x added a commit that referenced this pull request Mar 10, 2026
almas-x added a commit that referenced this pull request Mar 10, 2026
hwbrzzl added a commit that referenced this pull request Mar 11, 2026
* chore: Update upgrade DB packages (#1374)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Internalize file rotation logic from goravel/file-rotatelogs (#1375)

* Initial plan

* Implement internal file rotation to replace goravel/file-rotatelogs

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Improve cleanup logic with glob pattern matching and add comprehensive tests

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Address code review feedback: fix trailing whitespace, add cleanup wait, and make tests deterministic

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* optimize

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: Bowen <hwbrzzl@gmail.com>

* chore: Update non-major dependencies (#1373)

* chore: Update non-major dependencies

* optimize

* optimize

* renovate/non-major-dependencies

* optimize

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Bowen <hwbrzzl@gmail.com>

* feat: [#726] Add HTTP server and client telemetry instrumentation [5] (#1326)

* add http middleware

* add http transport

* optimise http telemetry package

* add test cases for Telemetry middleware

* add auto instrumentation configs

* add config to enable Telemetry for http clients

* add docs for config facade

* add ConfigFacade nil warning

* disable default telemetry

* optimise transport

* add kill switch for instrumentation

* update the stubs

* remove unnecessary handler

* optimise log instrumentation

* move route registration in the end

* lazily initialize middleware and transport to work with new application_builder

* optimise channel test

* optimise channel test

* accept telemetry facade as an input instead of using global instance

* use a callback to resolve the telemetry facade instance

* optimise grpc handler to remove usage of telemetry and config facade

* optimise the grpc handler

* use telemetry transport if enabled

* optimise http auto instrumentation

* optimize log test cases

* optimise

* optimise

* optimise

* optimise

* revert PR#1357

* fix log test cases

* revert GRPC changes

* optimise middleware

* remove zipkin trace driver

* correct GRPC enable condition

* correct http transport enable condition

* correct log channel enable condition

* update go mod

* fix test cases

* fix test cases

* fix test cases

* fix tests

---------

Co-authored-by: Bowen <hwbrzzl@gmail.com>

* chore: optimize runner tests for Windows (#1377)

Co-authored-by: Bowen <hwbrzzl@github.com>

* chore: Update non-major dependencies (#1378)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix: runner stuck (#1381)

* fix: runner stuck

* optimize

* optimize

* optimize

* optimize

* optimize

* chore: Update non-major dependencies (#1382)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* feat: [#849] Add table support to the console context (#1380)

* add table function in cli

* add test cases for new method Table

* update mocks

* add column level styles

* fix lint error

* move style vars to new file

* add GlobalHuhTheme

* update go mod tidy

* feat: [#546] artisan command up and down to set website Maintenance mode (#1198)

* Add up and down command

* Use file helper to create files and add unit tests

* Fix the foundation.App dependency

* Use support/path instead of foundation.App

* Add unit test case

* Add some missing checks

* Add more missing checks

* One more

* Fix tmpfile path

* Use T.TempDir instead of os.TempDir in tests

* Add reason to the down command

* Add option to the tests

* Change the maintenance file name

* close created file handles

* Defer abort

* Fix the linter issue

* Add more options to the down command

* Use options

* Check for maintenance mode respond

* Fix down_command_test

* Add more unit test cases

* Fix more lint issues

* Fix lint issue

* fix tests

* fix tests

* fix tests

* fix tests

* Address PR comments

* Fix check_for_maintenance_test

* Check Aborts in check_for_maintenance

* Rename check_for_maintenance_mode

* Fix and Write more unit tests for check_for_maintenance_mode middleware

* fix down command

* Fix down_command_test

* Fix up_command

---------

Co-authored-by: Bowen <hwbrzzl@gmail.com>

* feat: Laravel-style Collection library (#1134)

Merges comprehensive Collection library with 100+ methods for data manipulation.

Implements both eager (Collection) and lazy (LazyCollection) evaluation strategies.
Closes goravel/goravel#566

Follow-up improvements tracked in: goravel/goravel#883

* Increase route module coverage for factory, provider, and runner wiring paths (#1384)

* Initial plan

* test(route): cover route factory and service provider paths

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* optimize

* optimize

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: Bowen <hwbrzzl@gmail.com>

* chore: optimize interface (#1389)

* Increase crypt module coverage by exercising AES key validation and failure paths (#1385)

* Initial plan

* test: add crypt AES edge-case coverage

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: improve readability of crypt key-length cases

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: address crypt PR feedback and simplify test structure

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: use configmock EXPECT style in TestNewAES

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: standardize config mock setup across aes tests

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Increase filesystem module test coverage for storage, service provider, and file facade paths (#1387)

* Initial plan

* Add filesystem application and provider coverage tests

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Polish filesystem coverage tests after review feedback

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Refine filesystem tests based on review feedback

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Apply review style suggestions in filesystem tests

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Increase mail module test coverage to 70%+ with focused unit tests (#1386)

* Initial plan

* test(mail): add focused unit coverage for application, options, and service provider

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): address review feedback on test specificity and readability

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): merge application unit tests and remove untyped mock matchers

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): deduplicate matchers and tighten queue job assertion

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): use AnythingOfType and any in application tests

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): derive bind callback type string from any signature

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): simplify bind callback type matcher

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): use AnythingOfType for template render expectations

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(mail): restore specific MatchedBy assertions per review clarification

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Increase hash module coverage with targeted tests for driver selection and service provider paths (#1388)

* Initial plan

* test: increase hash module coverage for driver and provider paths

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: cover hash provider and driver selection paths

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: address hash module review feedback

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: merge hash module tests into application_test

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: use EXPECT API in hash service provider test

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: use any alias in hash singleton callback test

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: add AI instructions file (#1393)

* Increase core framework coverage via targeted service provider and builder-path tests (#1391)

* Initial plan

* Add service provider coverage tests for core modules

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Expand core module coverage tests and validate suite

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Revert unintended test module dependency drift

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Address actionable PR review suggestions in coverage tests

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Finalize review feedback handling

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Revert unintended tests module dependency updates

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Tighten provider test matchers per review feedback

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* Increase infrastructure module coverage by adding Process/Packages/Log/Schedule tests and fixing errors.As target forwarding (#1392)

* Initial plan

* test(errors): cover As/Unwrap/Ignore/Join edge cases

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(errors): use wrapped error in As coverage test

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module file changes

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: add coverage for process packages log schedule modules

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: refine schedule coverage tests per review

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module dependency updates

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: assert boot no-panic in log and process providers

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: finalize review feedback responses for boot test assertions

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module file changes

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: expand service-layer coverage across Event, Queue, gRPC, Cache, and Console (#1390)

* Initial plan

* test(event): cover service provider and queued listener dispatch paths

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: finalize event coverage validation

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module dependency updates

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test(event): address review suggestions for robust command matching and queue error path

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: finalize PR feedback follow-up

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module file updates

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: add focused coverage cases for queue grpc cache console

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: clarify queue nil args in empty chain test

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module dependency updates

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* test: avoid nil context in console usage-error test

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: finalize lint ci fix validation

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: revert unintended tests module dependency drift

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: Update non-major dependencies (#1399)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore: add generate flag to artisan build command (#1402)

* feat: [#911] add query context accessor (#1401)

* chore: optimize agents (#1396)

* chore: optimize agents

* optimize

* address comments

* Initial plan

* fix: backport migrate rollback default handling to v1.17.x

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* chore: align branch tree with v1.17.x before backporting rollback fix

Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>

* optimize

* optimize

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Bowen <hwbrzzl@gmail.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: krishan kumar <84431594+krishankumar01@users.noreply.github.com>
Co-authored-by: Bowen <hwbrzzl@github.com>
Co-authored-by: Mohan Raj <praem1990@gmail.com>
Co-authored-by: Ahmed M. Ammar <ahmed3mar@outlook.com>
Co-authored-by: 耗子 <haozi@loli.email>
Co-authored-by: ALMAS <almas.cc@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants