Skip to content

Conversation

@miparnisari
Copy link
Contributor

@miparnisari miparnisari commented Apr 21, 2025

In preparation for #478, adding more tests to ensure we don't break existing behavior.

@miparnisari miparnisari requested a review from tstirrat15 April 21, 2025 23:16
@miparnisari miparnisari force-pushed the more-validation-tests branch 2 times, most recently from 5d7e394 to 859ae4a Compare April 21, 2025 23:21
var errWithSource spiceerrors.WithSourceError
if errors.As(err, &errWithSource) {
ouputErrorWithSource(validateContents, errWithSource)
outputErrorWithSource(toPrint, validateContents, errWithSource)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to hit this code path yet


// Output errors
outputDeveloperErrorsWithLineOffset(validateContents, devErrs.InputErrors, schemaOffset)
outputDeveloperErrorsWithLineOffset(toPrint, validateContents, devErrs.InputErrors, schemaOffset)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to hit this code path yet

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC this and the other are some combination of compilation errors and typesystem errors. Grepping for DeveloperErr and ErrorWithSource should probably turn something up.

Copy link
Contributor

Choose a reason for hiding this comment

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

JK you've already got some of that kind of thing in there...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Covered in test here: #494

@miparnisari miparnisari force-pushed the more-validation-tests branch from 859ae4a to b8f5a75 Compare April 21, 2025 23:28
@miparnisari miparnisari marked this pull request as ready for review April 21, 2025 23:29
Copy link
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

I like where this is headed


// Output errors
outputDeveloperErrorsWithLineOffset(validateContents, devErrs.InputErrors, schemaOffset)
outputDeveloperErrorsWithLineOffset(toPrint, validateContents, devErrs.InputErrors, schemaOffset)
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC this and the other are some combination of compilation errors and typesystem errors. Grepping for DeveloperErr and ErrorWithSource should probably turn something up.


// Output errors
outputDeveloperErrorsWithLineOffset(validateContents, devErrs.InputErrors, schemaOffset)
outputDeveloperErrorsWithLineOffset(toPrint, validateContents, devErrs.InputErrors, schemaOffset)
Copy link
Contributor

Choose a reason for hiding this comment

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

JK you've already got some of that kind of thing in there...

@miparnisari miparnisari merged commit 5bd373c into main Apr 22, 2025
11 checks passed
@miparnisari miparnisari deleted the more-validation-tests branch April 22, 2025 04:53
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants