test(cosmovisor): improve TestParseUpgradeInfoFile with substring assertions

[odeke-em]

In trying to investigate the flake reported in #18073, this change adds the context of the failing test's filepath. This change allows us to tighten the tests by using assertions on the error instead of just checking that it is non-nil.
Also one of the tests that claimed it was empty actually had a space/newline which meant that it wasn't testing the right expectations.

Updates #18073

Summary by CodeRabbit

• Refactor: Enhanced error reporting in the parseUpgradeInfoFile function. Now, if the upgrade-info.json file is empty, the error message will include the filename, making it easier to identify the source of the issue.
• Test: Improved the TestParseUpgradeInfoFile function. The test cases now expect specific error messages, providing more precise error reporting during testing.

[coderabbitai]

Walkthrough

The changes enhance error reporting in the parseUpgradeInfoFile function and its corresponding test cases. The function now returns a more specific error message when the upgrade-info.json file is empty. The test cases have been updated to check for these specific error messages.

Changes

File	Summary
tools/cosmovisor/scanner.go	The parseUpgradeInfoFile function now returns a more specific error message when the upgrade-info.json file is empty.
tools/cosmovisor/scanner_test.go	The TestParseUpgradeInfoFile function has been updated to check for specific error messages. The expectErr field has been changed from a boolean to a string.

🐇💻

In the land of code, where the errors hide,

A rabbit hopped and took a stride.

With a tweak and a test, the bugs were chased,

Now the code runs with grace, not haste.

So let's celebrate this change, so neat,

For making our software more complete! 🎉

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai help to get help.
• @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• 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/coderabbit-overrides.json

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 894bebc and 6389f6d484aecbdf413adf4c337ee478f57e0331.

Files ignored due to filter (1)

• tools/cosmovisor/testdata/upgrade-files/f3-empty.json

Files selected for processing (2)

• tools/cosmovisor/scanner.go (1 hunks)
• tools/cosmovisor/scanner_test.go (2 hunks)

Files skipped from review due to trivial changes (1)

• tools/cosmovisor/scanner_test.go

Additional comments: 1

tools/cosmovisor/scanner.go (1)

• 192-192: The error message has been improved to include the filename, which will provide more context when debugging. This is a good practice as it helps to quickly identify the source of the problem.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 894bebc and b2a4a574a511c19900e956e083c86b525e8467d7.

Files ignored due to filter (1)

• tools/cosmovisor/testdata/upgrade-files/f3-empty.json

Files selected for processing (2)

• tools/cosmovisor/scanner.go (1 hunks)
• tools/cosmovisor/scanner_test.go (2 hunks)

Files skipped from review due to trivial changes (1)

• tools/cosmovisor/scanner_test.go

Additional comments: 1

tools/cosmovisor/scanner.go (1)

• 192-192: The error message has been improved to include the filename when the upgrade-info.json file is empty. This change provides more context and makes it easier to identify the source of the error.

- return upgradetypes.Plan{}, errors.New("empty upgrade-info.json")
+ return upgradetypes.Plan{}, fmt.Errorf("empty upgrade-info.json in %q", filename)

Committable suggestion (Beta)

	if len(f) == 0 {
		return upgradetypes.Plan{}, fmt.Errorf("empty upgrade-info.json in %q", filename)
	}

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 331e106 and 4358c2a0cdfb4960082578fdc1082237aaab5b49.

Files ignored due to filter (1)

• tools/cosmovisor/testdata/upgrade-files/f3-empty.json

Files selected for processing (2)

• tools/cosmovisor/scanner.go (2} hunks)
• tools/cosmovisor/scanner_test.go (2} hunks)

Files skipped from review due to trivial changes (1)

• tools/cosmovisor/scanner_test.go

Additional comments: 2

tools/cosmovisor/scanner.go (2)

• 192-192: The error message for an empty upgrade-info.json file now includes the filename, providing more specific context about the error source. This is a good practice as it helps in debugging by providing more detailed error messages.

• 210-210: The function now returns nil instead of err as the error. This change is correct if we assume that any error would have been returned earlier in the function. However, it's important to ensure that all error cases are indeed handled before this point in the function.

- return upgradePlan, err
+ return upgradePlan, nil

Committable suggestion (Beta)

		upgradePlan.Name = strings.ToLower(upgradePlan.Name)
	}

	return upgradePlan, nil
}

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between e0d84b7 and 940b38d83e6e085b8ffdcd96cc86a075d5510252.

Files ignored due to filter (1)

• tools/cosmovisor/testdata/upgrade-files/f3-empty.json

Files selected for processing (2)

• tools/cosmovisor/scanner.go (2} hunks)
• tools/cosmovisor/scanner_test.go (2} hunks)

Files skipped from review due to trivial changes (1)

• tools/cosmovisor/scanner_test.go

Additional comments: 2

tools/cosmovisor/scanner.go (2)

• 192-192: The error message for an empty upgrade-info.json file now includes the filename, providing more context about the error source. This is a good practice as it helps in debugging by providing more specific information about the error.

• 210-210: The function now returns nil instead of err in the case where there is no error. This is a good practice as it makes the function's behavior more predictable and consistent. It's important to always return nil when there's no error, rather than a potentially uninitialized error variable.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between a755fef and 8537b5a.

Files ignored due to filter (1)

• tools/cosmovisor/testdata/upgrade-files/f3-empty.json

Files selected for processing (2)

• tools/cosmovisor/scanner.go (2} hunks)
• tools/cosmovisor/scanner_test.go (2} hunks)

Files skipped from review due to trivial changes (1)

• tools/cosmovisor/scanner_test.go

Additional comments: 2

tools/cosmovisor/scanner.go (2)

• 192-192: The error message for an empty upgrade-info.json file now includes the filename, providing more context about the error source. This is a good practice as it helps in debugging by providing more specific information about the error.

• 210-210: The function now returns nil instead of err in the case where there is no error. This is a good practice as it makes the code more readable and understandable. It's clear that if the function reaches this point, no error has occurred.

[alexanderbez]

👍

[odeke-em]

Looks like we've got 2 odd test failures. Am gonna whip out my laptop and examine them.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 21e69ba and 42c90bf.

Files ignored due to filter (1)

• tools/cosmovisor/testdata/upgrade-files/f3-empty.json

Files selected for processing (2)

• tools/cosmovisor/scanner.go (2} hunks)
• tools/cosmovisor/scanner_test.go (2} hunks)

Files skipped from review due to trivial changes (1)

• tools/cosmovisor/scanner_test.go

Additional comments: 2

tools/cosmovisor/scanner.go (2)

• 192-192: The error message has been improved to include the filename of the problematic file. This is a good change as it provides more context to the error and can help in debugging.

• 210-210: The function now returns nil instead of err in the case of no errors. This is a good change as it makes the error handling more explicit and clear.