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

Print representation of structs in Emacs 26 breaks tests #2133

Closed
xiongtx opened this issue Dec 11, 2017 · 6 comments
Closed

Print representation of structs in Emacs 26 breaks tests #2133

xiongtx opened this issue Dec 11, 2017 · 6 comments

Comments

@xiongtx
Copy link
Member

xiongtx commented Dec 11, 2017

Some tests are failing when run against Emacs 26 due to the new #s struct print syntax, which seems to be due to the use of records to represent structs.

@gonewest818
Copy link
Contributor

gonewest818 commented Dec 15, 2017

I'm curious about this.

FAILED: Expected `(nrepl-bdecode "lli1ei2ei3eeli5ei6eee")' to be `equal' to 
`([cl-struct-queue nil nil] . [cl-struct-nrepl-response-queue (((1 2 3) (5 6))) (((1 2 3) (5 6))) nil])', 
but instead it was `(#s(queue nil nil) . #s(nrepl-response-queue (((1 2 3) (5 6))) (((1 2 3) (5 6))) nil))'
which does not match because: (car (different-types #s(queue nil nil) [cl-struct-queue nil nil])).

It doesn't look like the reader supports #s syntax for arbitrary structs, so how does one specify the correct match for that test?

Would you be okay with constructing the match on the fly, as in

    (it "decodes queues"
      (expect (nrepl-bdecode "lli1ei2ei3eeli5ei6eee")
              :to-equal (cons
                         (make-queue)
                         (let ((n (nrepl-response-queue)))
                           (queue-enqueue n `((1 2 3) (5 6)))
                           n))))

I don't like mixing tests (the nrepl-decode and the constructors for those structures) but it does work.

@xiongtx
Copy link
Member Author

xiongtx commented Dec 15, 2017

The Emacs 26 reader should support #s for structs. Is that not the case?

@gonewest818
Copy link
Contributor

gonewest818 commented Dec 15, 2017 via email

@xiongtx
Copy link
Member Author

xiongtx commented Dec 15, 2017

#s notation was supported only for hash-tables

Funny you should mention it--I actually asked about this on emacs-devel. Seems the decision by Stefan Monnier to use #s (which corresponds to #s for structures in CL) was a controversial one.

Ultimately I think Stefan made the right choice--closer alignment w/ CL is a good thing.

@gonewest818
Copy link
Contributor

Ok this works in emacs 26.0.90

 (expect (nrepl-bdecode "lli1ei2ei3eeli5ei6eee")
                  :to-equal (cons
                             #s(queue nil nil)
                             #s(nrepl-response-queue (((1 2 3) (5 6))) (((1 2 3) (5 6))) nil)))))

But it’s not valid code for older versions of emacs, so presumably a macro or something is needed to conditionally compile this new test canonical only when the emacs version is 26 or greater.

@xiongtx
Copy link
Member Author

xiongtx commented Dec 17, 2017

Building structs on-the-fly seems to be the best way to avoid this problem.

xiongtx added a commit to xiongtx/cider that referenced this issue Dec 18, 2017
Fixes clojure-emacs#2133

In Emacs 26, the print representations of structs changed from `[cl-struct ...]`
to `#s(...)`. This broke tests that compare structs against print
representations. Building structs dynamically resolves this.
bbatsov pushed a commit that referenced this issue Dec 18, 2017
Fixes #2133

In Emacs 26, the print representations of structs changed from `[cl-struct ...]`
to `#s(...)`. This broke tests that compare structs against print
representations. Building structs dynamically resolves this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants