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

Include Advanced Extra-Credit tests #129

Closed
stephnr opened this issue May 17, 2015 · 13 comments
Closed

Include Advanced Extra-Credit tests #129

stephnr opened this issue May 17, 2015 · 13 comments

Comments

@stephnr
Copy link

stephnr commented May 17, 2015

Perhaps we can consider adding new tests where the user can complete several tests that have a much higher difficulty. For example:

  1. Arrays -> Sorting algorithms
  2. Strings -> Regex
@kadamwhite
Copy link
Contributor

I feel like this is a logical extension of what we offer currently; I'd be in favor of extending this material to cover more intermediate concepts like you describe. My only caution would be that we do so in a way that does not jeopardize the approachability of the repository for a beginner; and that, as the problems we "assess" gradually become more abstract, we take care to indicate when multiple solutions to a given abstract problem can be thought of as equally viable depending on context. The more abstract the challenge the more nuanced the potential solutions, which can be difficult to encapsulate in unit-test driven code.

@ashleygwilliams
Copy link
Collaborator

@Stephn-R there is a regex section already, would this be an extension of that?

i do agree with @kadamwhite, but as an example, #128 i think is a valuable extension. i'm not sure i love "extra-credit" as the term, but we do need a strategy for extending what we already have. would love to hear more discussion, especially @jmeas and @rmurphey ..

@jacobroufa
Copy link

One possibility, as mentioned in a comment in #128 is to make a separate repo a la js-assessment-advanced. I don't think that's wholly a bad idea. I personally would like to know of more assessment-style repositories for my own growth and kata-style learnings, so maybe documenting other repos (including ones spun off of this) could be a part of #19? This conversation is definitely going somewhere though!

I feel like this is one of those more abstract problems that may have several equally viable solutions presented based on the context, as @kadamwhite eluded to earlier... 😆

@stephnr
Copy link
Author

stephnr commented May 20, 2015

@kadamwhite @ashleygwilliams @jacobroufa @everyone

I 1000% agree with everyone. This would be better off as an extension library/npm module we could include.

Any thoughts yet on how to make js-assessment extensible?

@richardartoul
Copy link

Hey @Stephn-R ,

I'm gonna copy and paste my comment from #128 :

"I'm all for modularity, however, I don't think that a package manager is the way to go. It adds (in my opinion) an unnecessary level of complexity and refactoring of the existing code base. The existing setup actually lends itself to namespacing and modularity quite well. Different test suites can use different runner.html and in addition if you take a look at the existing package.json file:

"scripts": {
"start": "node index.js",
"test": "mocha -R spec 'tests/app'"
},

it easy to see how we could do something like this:

"scripts": {
"start": "node index.js",
"test-beginner": "mocha -R spec 'tests/app-beginner'"
"test-intermediate": "mocha -R spec 'tests/app-intermediate'"
},

In terms of the js-assessment already being fairly long, well, I don't think its too much to expect employers using the js-assessment test to pick and choose which tasks they want prospective hires to undergo. Its already easy to turn certain tests on and off by simpling adding an "x" in front of the corresponding mocha describe statement.

I think one possible temporary solution (because it will take time to figure out which tests go together, what is intermediate, what is advanced, what we want to include in the core functionality etc) is to add a "additional tests" section. Here we an add more nuanced/advanced tests that we think are useful. If someone wants to use these tests, they can simply execute npm test-additional and people can also pick and choose which of the additional tests get run using the mocha "x" feature (we can default them all to off). This area could also serve as a "testing ground" to see which tests should make it into the core set of tests that js-assessment currently employers. Thoughts?"

@stephnr
Copy link
Author

stephnr commented May 20, 2015

@richardartoul, I fail to see how modularizing the tool is a challenge. Thankfully, @ashleygwilliams and @rmurphey have done an _amazing_ job keeping the code very simple.

Now that is not to say that your advice comes without merit. It does make it worthwhile to add more to the package.json scripts but, as @ashleygwilliams and @rmurphey can chime in on, I believe that the focus is to not modify the original project as much as possible and to simply add extensible content via other projects.

I will spend some time looking into creating a very simple extensibility library, but for the most part, I would recommend that you _not_ limit your mindset of what is or is not capable or compatible. These assessment tests serve the purpose of assisting other groups and organizations with a unified approach to testing ones performance with Javascript. As such, it comes with a degree of expectation that those who are developing the application itself are revered as experts in Javascript and can perform as such.

@richardartoul
Copy link

@Stephn-R I'm not suggesting that creating a modularity tool is too much of a challenge, I was simply pointing out that, since like you said @ashleygwilliams did a great job of keeping the codebase clean, there are already great structures in place that lend themselves to modularity without forcing the user to deal with a package manager. I was emphasizing simplicity from the users standpoint, not the developers :)

That being said, I was not aware that it was @ashleygwilliams goals to avoid modifying the original project. The readme stills lists this:

"Submit a pull request! The tests are currently loosely organized by topic, so you should do your best to add tests to the appropriate file in tests/app, or create a new file there if you don't see an appropriate one. If you do create a new file, make sure to add it to tests/runner.js, and to add a stub for the solution to the corresponding file in app/. Finally, it would be great if you could update the answers as well."

so I thought it was very much still a work in progress. If an extensibility library is the way the team wants to go, I'm happy to write tests that way as well.

@rmurphey
Copy link
Owner

Hi there -- I'm the original creator of this, and just wanted to weigh in. @ashleygwilliams has been extremely generous in finding the time to take this project on, addressing old pull requests and bringing the project up to modern standards. The portion of the README that you quoted, though, is all me.

I am all for seeing more tests contributed, but I'm wary of categorizing tests as beginner, intermediate, or advanced. The definitions of those terms vary widely, and I much prefer the current self-guided situation. The current situation also means that companies using this test have to actually understand what's being asked, vs. just saying "here, take the intermediate tests please." The original set of tests were, loosely, organized in order of increasing difficulty -- that is, tests early in a file tend to be easier than tests later in a file. I'd encourage sticking with that pattern, but I would be 👎 on further classification.

@ashleygwilliams
Copy link
Collaborator

hey everyone!

so first off, thanks for all the comments! i think what i might be leaning towards doing here is creating a app\contrib directory where what we can call a "mildly curated" set of community contributions can live. we'll come up with a naming scheme to keep things semi-standard, topic rather than level based.

i see contrib tests primarily being tests that cover a new breadth of content than the current set. if the depth of content is what is to be updated that would be a pull directly on the category file that already exists. i agree with @rmurphey about the cons of adding a level tag and could go on at length about others.

when we go to release new major versions of js-assessment, i imagine that we can make the decision to add/remove new categories from contrib to the core. minor versions will be for adding depth to an already existing category.

long run i would like to build a package like functionality. i'd also like to create a fully in browser experience. both of those experiences would make it easier to discover content that might otherwise get lost in a large app/contrib. however, maintaining the usefulness and integrity of the 2 experiences we already have (browser tests/node tests) is key at the moment.

thoughts? i'd like to have this sorted by the end of the week so we can clean up all the PRs and issues :)

thanks again for all the dialogue! really love that ya'll care enough to contribute ❤️ ❤️ ❤️

@richardartoul
Copy link

@ashleygwilliams sounds like like a great plan, good mix of short-term practicality and long-term goals.

@stephnr
Copy link
Author

stephnr commented May 22, 2015

Right on point @ashleygwilliams , I say lets go with it

@jamesplease
Copy link
Collaborator

@ashleygwilliams , I think you've got a great plan. The contributing guide might shed more light on how we decide what goes in core and what goes in contrib, which would help me better understand the distinction. In Marionette, we had a 'contrib' section for ideas for UI patterns, and it got messy really fast. Since we evaluated those things less, we ended up having a large number of patterns that...I wouldn't recommend anyone use. I'd want us to make sure we don't find ourselves in the same situation here!

however, maintaining the usefulness and integrity of the 2 experiences we already have (browser tests/node tests) is key at the moment.

+50000

@rmurphey
Copy link
Owner

I like the idea of a contrib/ directory for adding more tests that don't make sense in the current test files. I do think it's fine for advanced tests to live alongside more basic ones, assuming the files generally start with simpler tests and move on to more complicated ones. If a lot of tests emerge in the contrib/ directory, then we can figure out how to incorporate them. In the meantime, I'm closing this issue due to age.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants