Skip to content

Add comprehensive unit tests for pkg/cmd/build/logger.go #4744

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

Closed
wants to merge 3 commits into from

Conversation

rberrelleza
Copy link
Member

Summary

This PR adds comprehensive unit tests for all functions declared in , achieving excellent code coverage and ensuring robust testing of the build logging functionality.

Changes

  • Created 44 test cases covering all 7 functions in logger.go
  • Achieved high code coverage: 100% for 6/7 functions, 96.6% for deployDisplayer
  • Comprehensive edge case testing including:
    • Error handling scenarios
    • Context cancellation
    • Missing vertex scenarios
    • JSON parsing errors
    • Different output modes (deploy, destroy, test)
    • Large context transfer scenarios

Functions Tested

    • 96.6% coverage (7 test cases)
    • 100% coverage (1 test case)
    • 100% coverage (8 test cases)
    • 100% coverage (11 test cases)
    • 100% coverage (5 test cases)
    • 100% coverage (3 test cases)
    • 100% coverage (4 test cases)

Testing Approach

  • Uses testify framework for assertions
  • Mock implementations for buildkit client dependencies
  • Table-driven tests for comprehensive scenario coverage
  • Edge case testing for error conditions and boundary cases
  • Code coverage measurement using go test and go tool cover

Coverage Results

github.com/okteto/okteto/pkg/cmd/build/logger.go:40:    deployDisplayer         96.6%
github.com/okteto/okteto/pkg/cmd/build/logger.go:100:   newTrace               100.0%
github.com/okteto/okteto/pkg/cmd/build/logger.go:108:   update                 100.0%
github.com/okteto/okteto/pkg/cmd/build/logger.go:149:   display                100.0%
github.com/okteto/okteto/pkg/cmd/build/logger.go:217:   isTransferringContext  100.0%
github.com/okteto/okteto/pkg/cmd/build/logger.go:223:   hasCommandLogs         100.0%
github.com/okteto/okteto/pkg/cmd/build/logger.go:227:   removeCompletedSteps   100.0%

Note

The only uncovered line in is the timeout case (line 70) which would require a 10-minute wait to test, making it impractical for unit testing without significant refactoring.


Created together with Okteto AI Agent Fleets 🤖

- Created 44 test cases covering all 7 functions in logger.go
- Achieved 100% code coverage for 6/7 functions (96.6% for deployDisplayer)
- Tests cover edge cases including error handling, context cancellation, and missing vertices
- Uses testify framework for assertions and mock implementations for dependencies
- Includes tests for deployDisplayer, newTrace, trace.update, trace.display, trace.isTransferringContext, trace.hasCommandLogs, and trace.removeCompletedSteps

Created together with Okteto AI Agent Fleets.
- Applied gofmt formatting fixes
- Fixed spacing and alignment issues
- Added missing newline at end of file
- Remove nil check before len() call on slice
- len() for nil slices is defined as zero in Go
- Fixes DeepSource warning about redundant nil check
Copy link

codecov bot commented Jul 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.34%. Comparing base (d7395da) to head (df4cc50).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4744      +/-   ##
==========================================
+ Coverage   48.91%   49.34%   +0.42%     
==========================================
  Files         356      356              
  Lines       29809    29809              
==========================================
+ Hits        14582    14709     +127     
+ Misses      14066    13936     -130     
- Partials     1161     1164       +3     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jLopezbarb
Copy link
Contributor

Hi @rberrelleza — appreciate the effort here. I’m closing this PR because the tests don’t catch behavior changes in the actual dependency, and our test style avoids if/else in favor of straightforward assertions for readability.

@jLopezbarb jLopezbarb closed this Jul 29, 2025
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