Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Put define-class tests into a single suite file #1274

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

jbouwman
Copy link
Collaborator

@jbouwman jbouwman commented Oct 1, 2024

Combine the programs and error messages in tests/parser-test-files/good-files/define-class.coal and tests/parser-test-files/bad-files/define-class.* into define-class.txt

A structured input format is documented in tests/parser-test-files/README.md. It supports:

  • reexecution of single cases
  • selective test disabling
  • optional error message checking
  • test numbering

And provides a place to put additional test metadata.
The tests now compile their inputs, in addition to parsing.

This incorporates changes from #1272 but only converts define-class tests and leaves others intact.

@jbouwman jbouwman requested a review from Izaakwltn October 1, 2024 23:31
Copy link
Collaborator

@Izaakwltn Izaakwltn left a comment

Choose a reason for hiding this comment

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

Looks good!

I left a number of comments about error messages in the bad tests- these are probably out of the scope of this PR, but I thought it was a convenient place to point them out.

tests/parser-test-files/define-class.txt Show resolved Hide resolved
--> test:3:33
|
3 | (define-class (C :a :b (:a :b ->)))
| ^ expected one or more type variables
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like a sketchy error message (outside of the scope of this PR)

error: Malformed functional dependency
--> test:3:23
|
3 | (define-class (C :a :b (-> :b :c)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This too, it should probably refer to the arrow, similarly to the => errors above .

(Once again, probably a separate PR)

--> test:3:14
|
3 | (define-class (C :a :b =>))
| ^^^^^^^^^^^^ missing class name
Copy link
Collaborator

Choose a reason for hiding this comment

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

(subsequent PR) this is a suboptimal error, the class name is right there

--> test:3:34
|
3 | (define-class (C :a :b (:a -> :b) :c))
| ^^ expected a list
Copy link
Collaborator

Choose a reason for hiding this comment

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

One more- I'm not sure what this error should be but this seems not ideal

--> test:3:31
|
3 | (define-class (Eq :a => (C :a) :b))
| ^^ unexpected form
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be better to have a "trailing" error, though might be awkward to implement

Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

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

a small nit on POSITION-IF

tests/loader.lisp Outdated Show resolved Hide resolved
@jbouwman jbouwman force-pushed the define-class-tests branch from cd24564 to 9e10c3a Compare October 2, 2024 15:54
Combine the programs and error messages in
tests/parser-test-files/good-files/define-class.coal and
tests/parser-test-files/bad-files/define-class.* into define-class.txt

A structured input format is documented in tests/parser-test-files/README.md. It supports:

- reexecution of single cases
- selective test disabling
- optional error message checking
- test numbering

And provides a place to put additional test metadata.

The tests now compile their inputs, in addition to parsing.
@jbouwman jbouwman force-pushed the define-class-tests branch from 9e10c3a to 32fb033 Compare October 2, 2024 15:55
@jbouwman jbouwman merged commit 2e69557 into coalton-lang:main Oct 2, 2024
33 checks passed
@jbouwman jbouwman deleted the define-class-tests branch October 3, 2024 16:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants