Skip to content

Add filesystem utilities. #37

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

Closed
wants to merge 7 commits into from
Closed

Conversation

Fuco1
Copy link
Contributor

@Fuco1 Fuco1 commented Jan 28, 2016

Questions I have:

  1. How do I update the documentation? I can clone the wiki repository but pushing to it works how exactly? I wouldn't want to ruin it :D
  2. Do you want the with-temp-fs macro prefixed with buttercup? Or some other name entirely, I'm not attached either way.

This addresses #16.

@jorgenschaefer jorgenschaefer added this to the v1.5 milestone Jan 28, 2016
@jorgenschaefer
Copy link
Owner

Wow, awesome. Do you have tests for this? :-)

You just push to the wiki, it's a normal git repo. If you cloned it, you can push to it.

As for the name, how about with-buttercup-filesystem to go with the file name?

@Fuco1
Copy link
Contributor Author

Fuco1 commented Jan 28, 2016

Right, I forgot to attach the tests. Will rebase.

@cocreature
Copy link

I just wanted to say that this is awesome and I can’t wait to use it! Also buttercup itself is awesome :)

@jorgenschaefer
Copy link
Owner

Thank you for the kind words! :-)

@Fuco1
Copy link
Contributor Author

Fuco1 commented Jan 31, 2016

In Emacs we have with-temp-buffer and with-temp-file so I'm still thinking maybe we should keep the prefix to make it more consistent. I would probably replace fs with filesystem so it's a bit less cryptic. What do you think?

You say in the test file

;;;;;;;;;;;;;;;;;;;;;
;;; Built-in matchers

;; Are tested in README.md

I don't see anything resembling inline tests there. How do I add tests for the new matchers?

@jorgenschaefer
Copy link
Owner

Oops, that was not updated when I moved the tests from README to docs/introduction.md – I'm fine with testing new matchers in an actual lisp file, or maybe in some docs/filesystem.md file?

@Fuco1
Copy link
Contributor Author

Fuco1 commented Jan 31, 2016

Now that I think of it, the matchers are implicitly tested in the "filesystem" tests, so I think we don't need separate tests.

@jorgenschaefer
Copy link
Owner

This looks good to merge to me, right? Could you squash the commits into a single one and rebase ontop of master? :-)

@jorgenschaefer jorgenschaefer modified the milestones: v1.5, v1.6 Mar 12, 2016
@Fuco1
Copy link
Contributor Author

Fuco1 commented Apr 10, 2016

Sorry for being late to the party.

This still needs some minor touches, but basically yes. I'll try to get this working.

@DamienCassou
Copy link
Contributor

What is the status?

@Fuco1
Copy link
Contributor Author

Fuco1 commented Nov 6, 2016

I rebased this on master, renamed temp to buttercup to make it clear where this comes from, cleaned up some formatting and added couple more tests.

I think this is ready for merge, you can review and then "squash and merge" through github.

Thanks for the patience and thanks @DamienCassou for the bump :D

@Fuco1
Copy link
Contributor Author

Fuco1 commented Nov 6, 2016

If anyone cared for some real use examples, here are some: https://github.com/Fuco1/dired-hacks/blob/master/tests/test-dired-filter.el (I have the code copied there but after this is merged I will clean it up).

@DamienCassou
Copy link
Contributor

I love this, thanks @Fuco1.

@DamienCassou
Copy link
Contributor

I would have liked to have this today. I need it now :-D.

@DamienCassou
Copy link
Contributor

After discussing with @Fuco1, I sent a PR for this patch to the assess project as I think it belongs there: phillord/assess#7. To me, buttercup is a DSL to do BDD which should offer syntactic sugar on top of existing test librairies.

@Fuco1
Copy link
Contributor Author

Fuco1 commented Nov 14, 2016

We still need to merge the matchers though. The mock filesystem code could then be removed from this patch.

@jorgenschaefer
Copy link
Owner

Considering buttercup and assess provide a lot of the same features (matchers are basically the same idea as most of assess), I don't know why buttercup would not use this code – or alternatively, buttercup should use more of assess. Opinions?

Otherwise, LGTM.

@Fuco1
Copy link
Contributor Author

Fuco1 commented Nov 18, 2016

Honestly, I'm not sure what is the best way to proceed. Until recently I wasn't aware of assess, so I pushed the code here. I would very much like to avoid duplication (similar situation happened with dash and seq). If assess has a chance to go into core, I think it is more reasonable long-term to concentrate there.

We both have less-than-stellar experience with emacs-devel, but I hear things are getting better. Are there any plans of moving buttercup into core as well?

There is also the issue of what the packages provide and what people expect. I would like to view buttercup as an "all-in" solution for testing, similar to how people view ert, which means either it should duplicate the matchers or bundle them somehow (also as inspiration, ecukes was the testing framework and the matchers/setups were in a different package, espuds). I still only have a vague idea of the goals of assess.

@Fuco1
Copy link
Contributor Author

Fuco1 commented Nov 18, 2016

We should also invite @phillord the author of assess.

@DamienCassou
Copy link
Contributor

I would love see buttercup build a great DSL on top of existing libraries such as assess and el-mock.

@phillord
Copy link

I've nicked some stuff from buttercup also -- assess-call was inspired by buttercup. It's a little unfortunate we both got far enough in, before we found out about the other project.

My intentionality for assess is very much a pile of fixtures, and utility functions. I don't really want to go the whole BDD route. But assess should, of course, be able to support that kind of development. So, I'd be very happy for these two projects to work together; for me, having buttercup on top of assess makes more sense than the other way around.

I do plan to put assess into core which has implications wrt to copyright. I would also like to try and fix up ert to solve some of the issues that buttercup addressess -- in particular that the output of ert is not really "pluggable" -- it's hard to integrate. I've also added assess-discover to make running tests easier.

@jorgenschaefer if you are interested in principle in pulling these two projects together, I guess, it's just a question of working out how.

@jorgenschaefer
Copy link
Owner

@DamienCassou wrote:

I would love see buttercup build a great DSL on top of existing libraries such as assess and el-mock.

Code re-use is always a means to an end, never the end in itself. Buttercup is meant to allow people to write readable and maintainable tests easier than ERT does. If existing libraries make that goal easier to reach, great. If not, then using them is no goal.

Buttercup could be a DSL for ERT, too. Sadly, ERT makes it hard to do that, so it was easier to write the code myself.

@phillord wrote:

if you are interested in principle in pulling these two projects together, I guess, it's just a question of working out how.

I'm always interested in cooperation when it is useful to both sides. :-D You write that you don't want to go the "whole BDD route", though, so I am not sure how much "pulling together" would be possible.

What I can see is some sort of trivial translation layer. As is, buttercup allows (expect <value> <matcher> <values…>) as a syntax. It might be possible to provide some form of translation macro on the Buttercup side, including default definitions, to translate these matchers into assess calls. E.g. :to-equal could translate directly to an asses= call. The only tricky part there that I can see would be negation. What would :not :to-equal translate to?

By the way, is there some kind of table of contents for the assess documentation, or ideally a single-page doc?

@phillord
Copy link

phillord commented Dec 7, 2016 via email

@jorgenschaefer
Copy link
Owner

I'd be happy with using assess with buttercup's frontend. Right now, it seems to me as if this would require mainly changing how expect and matchers work internally. I could be wrong.

I do not have the time to work on this, though, so if this is going to happen, someone else will have to put in the work I'm afraid.

@phillord
Copy link

phillord commented Dec 12, 2016 via email

@jorgenschaefer jorgenschaefer modified the milestones: v1.6, v1.18 May 20, 2017
@DamienCassou
Copy link
Contributor

Any news regarding this topic?

@phillord
Copy link

phillord commented Jul 3, 2017 via email

@bbatsov
Copy link

bbatsov commented Jul 7, 2018

Seems to me that probably @Fuco1's work should be merged in light of the lack of progress of the assess front.

@Fuco1
Copy link
Contributor Author

Fuco1 commented Jul 7, 2018

@bbatsov This was merged some time ago phillord/assess#7 I'm using it in dired-hacks tests along with buttercup as runner.

@phillord
Copy link

phillord commented Jul 7, 2018

@bbatsov When I send "I haven't had time", I meant for the assessing (pun!) whether buttercup could be implemented through assess. Sorry for lack of clarity.

@bbatsov
Copy link

bbatsov commented Jul 7, 2018

@Fuco1 Is it documented somewhere how to use buttercup with assess matchers?

I guess this PR should be closed if that's not going to be merged in buttercup itself.

@Fuco1
Copy link
Contributor Author

Fuco1 commented Jul 8, 2018

@bbatsov I'm only using the filesystem stuff ATM like this https://github.com/Fuco1/dired-hacks/blob/master/tests/test-dired-filter.el#L62 I don't know about the rest.

Yes, I think we can close this.

@Fuco1 Fuco1 closed this Jul 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants