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

Launching tests from a sub-project that doesn't have a gitroot #96

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paulRbr
Copy link

@paulRbr paulRbr commented Mar 6, 2014

Hello!

I was trying to launch my test suites of a yui project with yogi lately, and had no tests launch. Therefore I took a look inside the test resolver inside yogi. I found out that the current code searches for tests in the <closest_gitroot>/src/ directory.

The problem here is that if you have sub-projects inside on git repo, it will search for test at the git root, but not at the package root. Therefor this PR searches for the closest package.json and resolves the base path from there. If no package.json is found, it folds back to the current code (the gitroot).

Let me know if this is you'd like to integrate into yogi.

@@ -433,7 +435,13 @@ mods = {
canBatch = true;
}
if (canBatch) {
base = path.join(git.findRoot(), '../src');
find(CWD, 'package.json', function(err, file) {
if (file) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like an async process, while the previous join() call was sync.

@caridy
Copy link
Member

caridy commented Mar 6, 2014

Also, all the tests are failing!

@paulRbr
Copy link
Author

paulRbr commented Mar 6, 2014

Indeed, I'll fix that.

Btw, the find method is sync as well

@paulRbr
Copy link
Author

paulRbr commented Mar 6, 2014

Not sure why the test didn't run. Jshint is reportying errors on a file that I didn't change..

@caridy
Copy link
Member

caridy commented Mar 7, 2014

@popox I can confirm what you just said. It seems that a new version of the linting pkg is more strict. We will have to fix that before looking into your PR.

I was able to reproduce this by removing the node_modules then npm install then npm test.

@paulRbr
Copy link
Author

paulRbr commented Mar 10, 2014

Indeed linting is much more strict, you can change the conf via a .jshintrc in the project root to ignore the new properties.

Juste to confirm, running

npm test -f

in order to pass the linting errors, makes the tests pass:

✓ OK » 84 honored (0.015s) 

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

Successfully merging this pull request may close these issues.

2 participants