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

Fix glob option of .yeti.json #778

Closed
wants to merge 1 commit into from
Closed

Conversation

okuryu
Copy link
Member

@okuryu okuryu commented May 23, 2013

It is now not use Globstar if you are also using the Globstar, because it would match up to file under node_modules directory.

[okuryu.local]([email protected])$ yeti
Found 297 files to test.
  Agent connected: Chrome (26.0.1410.65) / Mac OS from 127.0.0.1
✓ Testing started on Chrome (26.0.1410.65) / Mac OS
✗ Script error: Uncaught ReferenceError: YUI is not defined
  URL: node_modules/yogi/defaults/tests/unit/index.html
  Line: 1
  User-Agent: Chrome (26.0.1410.65) / Mac OS
✗ Script error: Uncaught ReferenceError: YUI is not defined
  URL: node_modules/yogi/defaults/tests/unit/index.html
  Line: 13
  User-Agent: Chrome (26.0.1410.65) / Mac OS

So I would like to push this against dev-master branch because this doesn't affect the source code and test code.

/cc @reid

It is now not use Globstar if you are also using the Globstar,
because it would match up to file under `node_modules` directory.
@triptych
Copy link
Contributor

@okuryu Looks like Travis had an issue with this PR, do you see the same test failure on your side?

https://travis-ci.org/yui/yui3/jobs/7430101#L1983

@okuryu
Copy link
Member Author

okuryu commented Jun 14, 2013

@triptych No. I think that doesn't matter my change and this Travis CI error.

@reid
Copy link
Contributor

reid commented Jul 25, 2013

@okuryu Thanks for submitting this patch and sorry for the delay in our response.

It looks like the proposed change would work inside the root level directory. However, the current .yeti.json file also uses the globstar pattern to limit tests to the current component.

Perhaps we can create another .yeti.json inside of src to workaround this problem? The one inside of src can be like the original except the basedir would be .. instead of .

@okuryu
Copy link
Member Author

okuryu commented Jul 25, 2013

@reid Thank you for your response!

It looks like the proposed change would work inside the root level directory. However, the current .yeti.json file also uses the globstar pattern to limit tests to the current component.

I assumed .yeti.json is only for running Yeti inside the root of repository.

Perhaps we can create another .yeti.json inside of src to workaround this problem? The one inside of src can be like the original except the basedir would be .. instead of .

How about enhance options of .yeti.json file? I have an image of this approach has general versatility than adding another file. Like this:

{
    "basedir": ".",
    "glob": "**/tests/unit/*.html",
    "ignore": "node_modules/**"
}

@reid
Copy link
Contributor

reid commented Aug 8, 2013

@okuryu Okay. I've opened yui/yeti#57 to get that feature into Yeti.

The use of two configuration files can be used to mitigate the problem in the meantime.

@okuryu
Copy link
Member Author

okuryu commented Aug 8, 2013

@reid Thanks for file a ticket!

okuryu added a commit to okuryu/yeti that referenced this pull request Aug 17, 2013
This option is intended to exclude globbed directories.
Fix yui#57. See also yui/yui3#778.
@ghost ghost assigned okuryu Dec 16, 2013
@triptych
Copy link
Contributor

@okuryu can you close this issue out if it's no longer needed given @reid 's tickets?

@okuryu
Copy link
Member Author

okuryu commented Apr 16, 2014

Yeah, this is no longer needed for me. Closing makes sense to me.

@okuryu okuryu closed this Apr 16, 2014
@okuryu okuryu deleted the yeti-json branch May 4, 2014 07:44
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.

3 participants