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

Switch Tests to Library Tests #366

Closed
guygastineau opened this issue Aug 3, 2019 · 18 comments · May be fixed by #373
Closed

Switch Tests to Library Tests #366

guygastineau opened this issue Aug 3, 2019 · 18 comments · May be fixed by #373

Comments

@guygastineau
Copy link
Contributor

guygastineau commented Aug 3, 2019

@kotp and I have discussed before the possibility of running our tests against functions from sourced libs instead of testing an executable script.

With the new mandate for stubs I think this is a good time to discuss whether or not we would prefer switching to this model.


Pros:

  • Allow us to standardize the API of exercises.
  • Allow easier static code analysis for the auto-mentor project (I've been meaning to work on that for us).
  • Encourage students to modularize code into functions (perhaps leading to better bashers)
  • Might make it easier for newcomers.

Cons:

  • Robs autonomy from the students.
  • More difficult to debug. Less savvy students might have trouble realizing how to write an interim implementation script that uses their lib, or they may not realize that they can source it into their interactive environment to play with their work. (That should be in docs or the readme's)
  • it could make things slightly more complicated for some students.

Well, this is a big question/decision.

I welcome viewpoints from any and all of our maintainers, mentors, and students 😀

@glennj
Copy link
Contributor

glennj commented Aug 3, 2019

As well, functions can open up exercises that require some state to be kept between tests, such as List Operations.

@guygastineau
Copy link
Contributor Author

Good point @glennj
I have wanted to hack that exercise into our track for a while. Been busy though ;)

Does that mean you are on board?

@kotp
Copy link
Member

kotp commented Aug 3, 2019

I would have thought that tests should be atomic, and not rely on "state" between tests.

@guygastineau
Copy link
Contributor Author

I think he might mean state between sub tests of multiple functions. That is the kind of state that based flow we can't currently test with our model.

@guygastineau
Copy link
Contributor Author

There would likely be setup and teardown functions for each of these series of subtests.

@guygastineau
Copy link
Contributor Author

So, after #370 is merged I will make a demo PR for two-fer that treats it like a library.

This will prove that - and show how - it works.

@glennj and @kotp it seems like you all are in favor of this change being applied to the track.

@guygastineau
Copy link
Contributor Author

@IsaacG I would like to know where you stand too.

We seem like the only 4 people who are currently active here, so I think it is completely safe to proceed if we are in agreement with each other.

@IsaacG
Copy link
Member

IsaacG commented Aug 4, 2019

So much for abstaining :)

I don't have a strong preference. bash isn't exactly build around APIs and importing functions feels a bit weird. Generally, in bash the only API you have is the execve() of the script. On the other hand, an API does make life loads easier to use and still provides a fairly similar API to an exec(). So I'm fine either way.

@guygastineau
Copy link
Contributor Author

Thank you for weighing in @IsaacG

I agree that bash has a lot of flexibility. I often write simple scripts with no functions. Especially wrapper scripts (like my /usr/bin/vi is emacsclient -nw "$@")

Like @glennj pointed out, it should actually make previously impossible to-test-exercises viable for our track. Also, enforcing the "public" interface will allow us to write static code analysis programs much more easily. It's not like we are getting smashed in the mentor queues, but how many times should any of us have to good solutions to the simpler exercises.

Anyway, thanks again for sharing your opinion. I think the other two are on board, but the first PR will be like an example where everybody can weigh in how the test file should look and what-not; so I wont hand down mandates or anything like I'm Zeus 🤣

@guygastineau
Copy link
Contributor Author

We might even be able to figure out how to do atomic benchmarks this way.

@guygastineau
Copy link
Contributor Author

Hello again,

I know that our discussion is not finished here (or maybe it has only just begun 😄), but I went ahead and tossed up #373 so that we could look at tangible code propositions for handling the conversion to library testing.

Please feel free to comment there about specifics. This issue is probably better for continuing the guiding discussion. Also, feel free to clone #373. It would probably be best to rebase local work to 1 commit for easy cherry picking later, but I wont force push to the branch, so It should be safe for us to collaborate on it.

@glennj
Copy link
Contributor

glennj commented Aug 6, 2019

Copying comment from PR to here:

I think this comment thread is getting to the heart of how we want to manage name-spacing in our bash libs.

  1. do we want any? i.e. let source just dump library functionality into the "global" scope
  2. use a toplevel function with library functions accessed as sub-commands, as suggested here
  3. use a function prefix, like I did in the forth example

Does anyone know of any common idioms or best practice guidelines?

@bkhl
Copy link
Contributor

bkhl commented Aug 14, 2019

With this method, maybe we can make the testing so simple it can just be included at the top of the test file?

I feel like eliminating the dependency on Bats would on the whole be a good thing, and reduce the barrier to entry.

@guygastineau
Copy link
Contributor Author

guygastineau commented Aug 14, 2019

@bkhl

Interesting idea. I certainly don't think that bats is the greatest testing tool, but a lot of our test suites have a lot of tests, and this won't change what we get from problem-specifications.

I am concerned that this would produce a lot of exercises with 150+ lines up top followed by 5 lines of student solution. I fear this would cause more confusion/greater barrier to entry.

@guygastineau
Copy link
Contributor Author

guygastineau commented Aug 14, 2019

Furthermore, not to sound harsh, but if someone struggles to install bats they probably are not ready for exercism.

Technical sophistication is an important skill that even a lot of CS graduates are missing these days.

I know there is a lot of effort going into making exercism more accessible to inexperienced coders, but I think the current stumbling blocks are appropriate.

@guygastineau
Copy link
Contributor Author

So, it appears like most of us are at least in support of using library exercises that test the functions using a :: namespacing method.

I suppose we can start rolling out new PRs to update exercises. I will do a few this week.

I am also working on a new test generator to ease the burden of this change and the burden of updating exercises.

I will post a link to the repo for it when it is ready.

@guygastineau
Copy link
Contributor Author

Hmm, so most of us have been to busy I think to make much headway on this.

I am trying to get my mind wrapped around everything that v3 implies. It seems like v3 might push away from the library tests. Well, on the one hand it might make it easier to implement, but on the other it probably changes that shelly feeling that I think we are supposed to facilitate better with the jump to v3.

I think some good discussion will help us all figure out what the best course of action is.


Regarding the jump to v3 I will have limited time this holiday break, and I will likely need to devote much of that time to getting the scheme track v3 ready. I am curious to know how available/interested folks are in the jump to v3 here.

@glennj
Copy link
Contributor

glennj commented Dec 7, 2021

This idea was implemented for the List Operations exercise. As there's been no push forward to change any of the other exercises, I'm closing this issue.

@glennj glennj closed this as completed Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants