WIP: Convert to Lib tests

[guygastineau]

This is not intended for merging at all, but rather it is intended to share ideas about how to convert our tests to library testing. That being said, if commits only ever involve 1 exercise, then we can easily cherry pick conversions where we agree, so they can be merged in their own PR.

For now, I just did two-fer initial PoC and triangle since it is a bit more relevant.

@glennj I played with the :: you used for name spacing in forth, but I could not get it to work for triangle. It kept returning an error status. Anyway, I will keep investigating that, but any investigation you feel like doing would be appreciated.

I noticed that nounset absolutely may not be used in the implementation files for lib testing. sourceing the file into bats, then pollutes the bats environment. Since bats relies on variables being unset sometimes it will not work. We can probably use something like bash -c "source triangle; equilateral 2 2 2" to keep from polluting the bats environment. We should make sure we discuss this ;)

closes #366

---

Reviewer Resources:

Track Policies

[guygastineau]

As I said over in #366 I wont force push to this, so feel free to clone and we can all work together on it.

I think local work on any given exercise should be rebased before pushing back up here to make cherry picking easier.

@kotp @glennj @IsaacG Let me know if you guys would prefer having this branch belong to our exercism repo outright. It might be nice if we could all just have our own version of this branch through our personal clones. Anyway, just let me know ;)

[guygastineau]

travis_fold:start:before_script [0Ktravis_time:start:1ae0802b [0K$ bin/fetch_configlet

gzip: stdin: unexpected end of file tar: Unexpected EOF in archive tar: Unexpected EOF in archive tar: Error is not recoverable: exiting now travis_time:end:1ae0802b:start=1565059734245936845,finish=1565060043499524003,duration=309253587158 [0K[31;1mThe command "bin/fetch_configlet" failed and exited with 2 during .[0m

Your build has been stopped.

~I have seen this happen before. Was it here? I think it is happening, because the files are .xz, but tar wants to treat them like gzip format due to an extension.~

EDIT:

I remember now that I just needed to rerun the travis tests, because sometimes stuff fails for no reason :worried:

[guygastineau]

Additionally, if these are bash libraries (they are not posix compliant) then they should have .bash as their suffix.

[kotp]

The invalid and wontfix labels are added just to remind not to merge.

[guygastineau]

Thank @kotp those labels are a good reminder.

Do you have an opinion on whether or not this project should be switched to an architecture wherein it is a direct branch of exercism/bash?

[kotp]

I think it is fine where it is, until it isn't. We can clone it down and work with it locally, as it is.

[glennj]

FYI I have not noticed any problems with function names containing :: in bash 3.2 on the mac

[guygastineau]

So, is it possible to get new count of our positions regarding namespaces? The only thing that still concerns me about using :: is that it bars the solution from being posix compatible if a student wishes to do so, but that is not a big deal in my opinion.

Still my vote would be for no namespaces, since students are unlikely to encounter that so much in wild (although it does exist).

[guygastineau]

Tally for namespacing:

Contributors	With Function	With ::	None
@guygastineau	:x:	:x:	:heavy_check_mark:
@glennj	:x:	:heavy_check_mark:	:x:
@IsaacG	:x:	:heavy_check_mark:	:x:
@kotp	:x:	:white_check_mark:	:x:

[glennj]

So, is it possible to get new count of our positions regarding namespaces? The only thing that still concerns me about using :: is that it bars the solution from being posix compatible if a student wishes to do so, but that is not a big deal in my opinion.

Since this is a bash-specific track and bash is not POSIX compatible, IMO don't care.

Still my vote would be for no namespaces, since students are unlikely to encounter that so much in wild (although it does exist).

This is like import library vs from library import * in python. My vote is yes, but I'll abide by majority.

@guygastineau what's "with Function" mean? Is that capitalizing the library function names?

[IsaacG]

I'll vote for yes namespace.

[guygastineau]

@IsaacG I assume you mean with :: from our earlier conversations?

[guygastineau]

@glennj I meant using a function that takes subcommands to act as the entry point for the library.

[IsaacG]

Yes, I'm in favor of using the :: for function namespaces.

[glennj]

ref #376

[kotp]

I feel like this is the Bash track, and so by definition not sh and therefore not required to be POSIX compliant. sh is a specification, and Bash is a superset of sh. I do agree that we should be doing the .bash extension, instead. And there should be a clear explanation for that in the introductory material handed to students.

It may be interesting to have a sh track as well, where strict compliance is expected, but that isn't this track. In recent times, I have noticed:

file $(which sh)
/bin/sh: symbolic link to dash

This seems to happen on Debian (and at least some derivatives). OpenBSD seems to use something a little more Korn-y.

So the practice of "portability" using !#/bin/sh is not an issue here, but could also be mentioned.

Instead, as this is the Bash track, we should use #!/usr/bin/env bash or at least specifically #!/bin/bash and let the program itself fail if Bash isn't available. Again, the focus being "This is the Bash track".

Just some thoughts as it regards my "vote" in Tally for Namespacing.

[glennj]

... I do agree that we should be doing the .bash extension, instead.

How is that going to affect the CI scripts? I guess we'll have to throw a little extra logic into bin/validate_exercises

[kotp]

... I do agree that we should be doing the .bash extension, instead.

How is that going to affect the CI scripts? I guess we'll have to throw a little extra logic into bin/validate_exercises

No direct answer to this, but no (long term) logic. All should be .bash in my opinion.

[MichaelDimmitt]

Really good discussion here 🔥

another thing worth noting, diffing is easy to attain in bats

by changing

@test "a name given" {
  [[ $BATS_RUN_SKIPPED == true  ]] || skip
  run two_fer Alice
  [[ $status -eq 0 ]]
  [[ $output == "One for Alice, one for me." ]]
}

printDiff(){
  echo output;
  echo "$output";
  echo ;
  echo expected;
  echo "$expected;
}

@test "a name given" {
  [[ $BATS_RUN_SKIPPED == true  ]] || skip
  run two_fer Alice
  expected=$(cat <<-END
One for Alice, one for me.
END
  )
  printDiff
  [[ $status -eq 0 ]]
  [[ "$output" ==  "$expected" ]]
}

[ErikSchierboom]

And as a side note: the test runner should also be updated. It might even be necessary to support both formats to not break old exercises. Did we decide how to handle such a situation @iHiD ?

[IsaacG]

This PR has seen no activity in 6 months. Is it still being used or should it be closed out?

[kotp]

Leave it be. It is draft and still has a place.