A Bit of Computer Science: Add automated tests for project assignments
Checks
- [x] This is not a duplicate of an existing issue (please have a look through our open issues list to make sure)
- [x] I have thoroughly read and understand The Odin Project Contributing Guide
- [x] Would you like to work on this issue?
Describe your suggestion
Proposed changes: Add automated tests using Jest for the following assignments in the CS section of the Full Stack Javascript path.
- [ ] Project: Recursion
- [ ] Iterative and recursive fibonacci functions
- [ ] MergeSort function
- [ ] Project: Linked List
- [ ] Node object
- [ ] Linked List object
- [ ] Project: HashMap
- [ ] HashMap object
- [ ] Project: Binary Search Trees
- [ ] Tree object
- [ ] Project: Knight Travails
- [ ] KnightMoves function
Reasoning: At present, there is a lot of guesswork that goes on with the learners' solutions for the assignments in the CS section. It is usually resolved and they're guided in the right direction, but only when they ask in the server about it. If someone mistakenly believes they implemented it correctly and moves on without asking, they miss out on knowing their mistakes. I've witnessed this happen often with other learners, and even experienced it myself especially in the BST and HashMap projects.
Having automated tests like we did in the earlier parts of the course with the javascript-exercises would clear up a bulk of the confusion regarding the requirements early on, and enable confidence in their solutions for learners.
We could incorporate these into the javascript-exercises repo, or create a new one - whichever is preferable.
I did check to see if I didn't miss any of the assignments, but if some did go under the radar please let me know.
I understand that it is a substantial amount of work upfront, so I'm willing to write them all myself if this does get green-lit.
Path
Node / JS
Lesson Url
https://www.theodinproject.com/lessons/javascript-recursion https://www.theodinproject.com/lessons/javascript-linked-lists https://www.theodinproject.com/lessons/javascript-hashmap https://www.theodinproject.com/lessons/ruby-binary-search-trees https://www.theodinproject.com/lessons/javascript-knights-travails
(Optional) Discord Name
ginned
(Optional) Additional Comments
No response
Heads up that the recursion project's two exercises will likely be addressed by https://github.com/TheOdinProject/curriculum/issues/27265 but those two exercises specifically are still up for discussion after the completion of the other exercises proposed.
If this one was greenlit, note that the same would have to be applied to the Ruby CS course.
Oh. Sorry, I completely missed that one.
Hmm, while I could hack something together for ruby, I am sure it wouldn't be an example of idiomatic ruby code.
We'll cross that bridge when we get there :D. Maybe someone could help out with the ruby side of things - if not then I'll get something going with inputs from other people.
Translation would likely be done by a Rubyist, e.g. the exercises to address the linked issue are being translated by Josh. Was just a heads up that that work would need doing on top of the JS stuff, should it be desired.
I'm not outright opposed to this or anything, but it does pose a couple of problems that I'd like people to think about and discuss:
- Current solutions to the project wouldn't feature the tests. How much would we care about this?
- The JavaScript projects currently have flexible instructions regarding how data structures are setup and created. Learners have the freedom to build them with factory functions or
classsyntax. This probably has to go away if you're writing tests? What way should be selected? Maybe mix and match between projects such that learners practice with both, or would this be messy? What should be done about existing project submissions not meeting new specifications around this?
And yes, we would want this written in Ruby as well because we like to maintain parity between the two paths on the DSA curriculum content. But like Mao said, that's fine if you wouldn't be comfortable doing this yourself. We can get a Rubyist to do it.
You raise good points.
-
For the first and last points regarding project submissions, honestly I'm not sure what would be the least disruptive way to go about this. I am in favour of keeping them though. We still have old submissions in other projects like Project Homepage when the requirements were different. In my opinion the tests and project submissions would serve different purposes - the tests would validate if the learner's code is correct or not, while past submissions would remain as a way to look at alternative approaches. So as long as we keep the instructions broadly the same, it should not cause issues.
-
As for the part about test setup. I think we should be able to maintain the current flexibility of the instructions regarding Classes/Factory functions. Our tests could unconditionally create the objects with the
newoperator, which when called with factory functions should not functionally change things as far as I know. This would allow them to work regardless of whatever approach was used in the solution. All that needs to be ensured is that they follow a stable interface, which the instructions already make clear with the function signatures. -
This does bring up another concern though, the tests are only able to check the public interface of the objects. Taking
HashMapsas an example, there would be some solutions where thehashmethod andloadFactorare private, which does not lend well to testing if the hashing or growing works properly. I'm not sure what to do here - assume that they work properly if the hashmap overall works properly? Or make explicit that they should be public just for testing purposes?
If I may throw in my 2c, I feel like these projects aren't quite as appropriate, or at least won't benefit from being converted to formal javascript-exercises than, say, the recursion exercises in review at the moment. Those ones are to replace exercises from external resources, some of which have very odd solutions with redundant code or stuff that only works for the one example shown but not any others. So those ones make sense to bring in-house, and they're not formal "projects".
The projects are more involved and a little open-ended in terms of implementation IMO to make significantly valuable tests for. How do you tell that a function is a class or factory? What if someone decides to use constructor syntax? They can be called without new without throwing errors, so calling without new and catching errors isn't really that waterproof.
The instructions in the linked list/BST are fairly self-explanatory IMHO and don't really need the effort of automated tests, and any that have common edge cases that aren't clear could perhaps be addressed by clarifying their wording individually. In other cases, like KT, perhaps more examples can be provided so people are confident in the result. Rather than just one or two examples, give a list of start-end squares and the min move counts for them - wouldn't take up much screen space to do that.
I opened this issue with the assumption that our tests would be able to have a single syntax that could accommodate the different ways in which learners could define their objects. Restricting the present flexibility in favour of testing feels counter-productive to me, and making minimal changes to the current instructions is the priority.
This is achievable as long as learners use the following methods to instantiate their objects
- Class Declarations
- Class Expressions
- Constructor Function Declarations
- Constructor Function Expressions
- Factory Function Declarations
- Factory Function Expressions
I created a test repo based on my idea with partial implementations of LinkedList through the various methods mentioned above, where a single test file works with all but one of them (anon-arrow-factory.js).
The only type of definitions new does not work with, is when arrow function expressions are in play, as they cannot be called as constructors
So syntax as such does not work:
// Arrow function expression
const LinkedList = () => {
let head = null;
let tail = null;
const append = (value) => {
const node = new Node(value);
if (head === null) {
tail = head = node;
return;
}
tail.nextNode = node;
tail = node;
};
// rest of the implementation...
return { append };
}
const list = new LinkedList(); // throws a TypeError
Unfortunately, even a single type of object definition not working will result in increased complexity and unreliability of tests. Not a fan of that. I am not aware of any solution to this issue that preserves the intended simplicity of the tests while also not restricting the learners.
In which case, I think going along with @MaoShizhong's solution of just making the driver scripts / self-checks more comprehensive would be ideal.
That said, I'm open to any other suggestions too.
JS annoying
I think I would also prefer to see this effort put into making the self checks more comprehensive where needed, since that will prevent the overhead associated with creating an automated solution.
But I would love more thoughts from you all and @TheOdinProject/ds-a
Tests would be invaluable for these projects.
I think we should add writing tests as project requirement instead, this would allow the learners to write their own tests. So for the JavaScript course, we can move the "A Bit of Computer Science" section after the "Testing JavaScript" and add testing as a requirement to each DSA project.
That is a good idea, it would incorporate testing while also not restricting users to a specific approach.
Does that include moving the Battleship project forward as well? I think that it's suitable as a capstone project for the JavaScript course. Putting "A Bit of Computer Science" back would put "Knight Travails" as the capstone project, which seems a bit underwhelming in terms of complexity, considering all that comes before it.
If that change is agreed upon, I think it would be more appropriate to keep Battleship project as the capstone project, maybe by moving it into the "Finishing Up with Javascript" Section, just before "Conclusion" lesson.
This issue is stale because it has had no activity for the last 30 days.
@gingkapls Maintaining Battleship as the capstone is perfectly doable, and what you proposed is essentially what's already done in the Ruby course (Chess is placed in the conclusion section): https://www.theodinproject.com/paths/full-stack-ruby-on-rails/courses/ruby#conclusion
I like the idea of having people write their own tests. I think it's a good opportunity to learn, as things tend to have pretty clear "X input yields Y output" blackbox type situations in DSA. And although not a primary concern because it's more "meta", it also eases our work and maintenance effort compared to writing tests for learners to use. That's something I'll bring to the team and get a larger discussion going on any potential issues.
Sorry it's taken a while to get this looked at more.
@JoshDevHub No worries! I know everyone's busy with all kinds of stuff, and we're all volunteers here - so all the effort is appreciated even if it takes a while. Thank you for your time ^^.