Skip to content
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

Fix to IsActive utility function in nodes package #608

Merged
merged 1 commit into from
Mar 7, 2025
Merged

Conversation

javuto
Copy link
Collaborator

@javuto javuto commented Mar 7, 2025

Adding tests to the utility function IsActive for nodes and small fix to compare times in absolute values.

@javuto javuto added the test Adding or changing tests label Mar 7, 2025
@javuto javuto requested a review from Copilot March 7, 2025 12:53
@javuto javuto self-assigned this Mar 7, 2025

Choose a reason for hiding this comment

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

PR Overview

This PR fixes the logic in the IsActive utility function for the nodes package and adds tests to validate its behavior.

  • Adjusted the time comparison in IsActive to use absolute values.
  • Added tests to cover scenarios with recent and stale node states.

Reviewed Changes

File Description
pkg/nodes/utils_test.go Added test cases for validating the IsActive function behavior
pkg/nodes/utils.go Updated time comparison logic to use absolute value for hours

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

pkg/nodes/utils.go:9

  • Consider adding input validation to ensure that the 'inactive' parameter is negative, as expected by the function logic.
func IsActive(n OsqueryNode, inactive int64) bool {

pkg/nodes/utils_test.go:11

  • Consider using a fixed timestamp (e.g., via time.Date) in tests to ensure deterministic behavior.
now := time.Now()
@javuto javuto merged commit 8fca4be into main Mar 7, 2025
52 checks passed
@javuto javuto deleted the fix-isactive-node branch March 7, 2025 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Adding or changing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant