Add doctests to pure functions and/or extract pure parts to new doc tested functions

[bglusman]

Currently most of our testing is integration tests, and even that's not very good after styling changes broke some of them, but I've been wanting to get some doc tests written and this seems like a good entry point for anyone new to the project to get familiar with codebase and help improve things!

[davearonson]

I'll take a whack at this. Don't know doctest very well yet, but want to. :-)

[bglusman]

Hey man! How you been? Any mixture of unit, doc tests and improved/increased feature tests are all welcome! Doc tests are neat but not an easy/perfect fit for all methods depending how much setup is needed. We have factories but I forget if they work in doc tests? Either way, I've just been too lazy/busy to beef up the testing so all help is welcome!

[davearonson]

Been fine; can put details in email if you want, or discuss at WeCamp if you can make it. :-)

As for adding tests, doctest will be my main focus for this as I specifically want to level up on that. (Love the concept, and I've told the local Elixir UG organizers I wanna give a talk on it... after I learn it.) Not sure I wanna do much more right now as all the overhead (Docker, Node, etc.) seems a bit overwhelming; I've learned the very basics of Docker and know what Node is but have studiously avoided it. ;-)

[davearonson]

Question about the README: it's not quite clear to me if the bullet points immediately under "Getting started with development" are all needed, or, especially since the last one says "ALTERNATIVELY", they are different ways of accomplishing the same thing.

Tried to do the first one (homebrew) but brew bundle said Error: Homebrew Bundle must be run under Ruby 2.3! -- while ruby -v said it was 2.3 (specifically 2.3.0p0). 😕 Did the mix stuff and it it seemed to succeed though. brew bundle barfing is probably why iex -S mix phx.server told me open_pantry/assets/node_modules/brunch/bin/brunch didn't exist, and to run npm install in the assets dir. Doing that made the next run of iex not complain, so yay. However, I'm still left wondering what else may have not gotten installed (or at least properly), which may come back to bite me later.

[bglusman]

I'll hit you up on slack to debug/discuss this, but I think you're ok now if it's running.

[bglusman]

I think Dave has dropped this so should be free for others to pick up, but correct me if wrong Dave!

[davearonson]

Sorry, my client finally came through with some funding so I've been focusing on that. I can work on it some tomorrow, but frankly I'd recommend splitting it into two, one for more unit tests (which would be good for someone who groks what this thing is sposta do), and one about doctests.

On the other claw, Tuesday is the last day of my current contract's latest Period of Performance, so if they don't come up with an extension by Wednesday, I'll be able to work on it more then. ;-)

[bglusman]

That's not a bad idea to split up tickets... We do need more unit tests. Sorry, didn't mean to rush you, after last slack message I thought you were maybe not able to

[davearonson]

Okay, I've gotten some spare time and waded through it to find a few good spots. Is davearonson@633b052 the kind of thing you're looking for? Pending reply, I'll continue in that vein.

[bglusman]

Oh, cool... yeah, that's possibly a bit TOO verbose/comprehensive for doc tests in my opinion, but good example of an easy to demonstrate and document pure function I'd forgotten about completely :-) I'd stick to 1-3 good examples per function, not trying to exhaustively test when it comes to doc tests, they're documentation first I think and unit test second, and having that much of the file be dominated by examples makes it a bit noisy. But overall good start, I'd pick a good demo subset of that and I think the PR is off to a good start! I'm also not sure the default_to method is even used anywhere, i think I may have copied this module from somewhere on the web like probably here https://gist.github.com/erikreedstrom/5104f5eea925cdece6e4 so ideally I should have cited that... not even sure it's really idiomatic Elixir these days do have this, but it is still convenient sometimes.

[davearonson]

Hmmm, according to ack, default_to is not used at all, so I'll chop it out.

[bglusman]

Also, could combine a bunch of those into fewer examples with e.g. Enum.all?([[], %{}, nil, false, ""], &Blank.blank?(&1))

[davearonson]

That would make it fewer lines, but IMHO less clarity. I'm in the midst of making it less "dominant" by making the present? doc just refer to the blank? doc. I've also added to the text above the examples.

[bglusman]

Well, the exact version could be a little different, but I like that it makes clear by example that basically this list of things are ALL the things that are ever Blank unless you extend protocol further, and then just need a short sample of examples of what are not blank... can be comments explaining also of course, but I think in this case being succinct is clearer. But take your pass at it however you want I guess :-)