Skip to content

Conversation

@ChrisBQu
Copy link
Collaborator

@ChrisBQu ChrisBQu commented Mar 7, 2025

Relevant issue(s)

Resolves #2568

Description

Validator functions currently were returning singular errors when encountered. With this change, they will instead return a list of errors that are encountered, by using the Join() function on the errors.

To accommodate this change, additional logic was added to some validator functions to check for nil values being passed in. This is to avoid and prevent nil pointer exceptions.

This is still in progress.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Specify the platform(s) on which this was tested:

  • Windows

@ChrisBQu ChrisBQu added the area/errors Related to the internal management or design of our error handling system label Mar 7, 2025
@ChrisBQu ChrisBQu added this to the DefraDB v0.17 milestone Mar 7, 2025
@ChrisBQu ChrisBQu self-assigned this Mar 7, 2025
@ChrisBQu ChrisBQu changed the title Validator functions join errors refactor: Make validator return joined errors Mar 7, 2025
@codecov
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 93.61702% with 9 lines in your changes missing coverage. Please review.

Project coverage is 78.56%. Comparing base (2fdb6ed) to head (2e03fe8).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
internal/db/definition_validation.go 93.62% 8 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3513      +/-   ##
===========================================
+ Coverage    78.46%   78.56%   +0.10%     
===========================================
  Files          391      391              
  Lines        36424    36466      +42     
===========================================
+ Hits         28577    28647      +70     
+ Misses        6144     6123      -21     
+ Partials      1703     1696       -7     
Flag Coverage Δ
all-tests 78.56% <93.62%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/db/definition_validation.go 95.34% <93.62%> (+1.35%) ⬆️

... and 17 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fdb6ed...2e03fe8. Read the comment docs.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@islamaliev islamaliev left a comment

Choose a reason for hiding this comment

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

LGTM!

@ChrisBQu ChrisBQu marked this pull request as ready for review March 17, 2025 15:25
@ChrisBQu ChrisBQu requested a review from a team March 17, 2025 15:26
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM, and is a nice improvement - thanks Chris :)

@ChrisBQu ChrisBQu merged commit 7d1daba into sourcenetwork:develop Mar 17, 2025
43 of 45 checks passed
ChrisBQu added a commit to ChrisBQu/defradb that referenced this pull request Apr 25, 2025
## Relevant issue(s)

Resolves sourcenetwork#2568

## Description

Validator functions currently were returning singular errors when
encountered. With this change, they will instead return a list of errors
that are encountered, by using the Join() function on the errors.

To accommodate this change, additional logic was added to some validator
functions to check for nil values being passed in. This is to avoid and
prevent nil pointer exceptions.

This is still in progress.

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Specify the platform(s) on which this was tested:
- Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/errors Related to the internal management or design of our error handling system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Return all errors from validators

3 participants