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

Blacklist for skipping tests or test folders in different environments. #110

Closed
wants to merge 3 commits into from

Conversation

ozymandias547
Copy link

This is a possible feature for ticket #107; It adds the ability to skip tests based upon the following blacklist in the settings.json:

"test_settings" : {
    "default" : {
        ...
        "blacklist" : {
              "files" : ["widgets/file1"],
              "directories" : ["pages", "templates"]
        },
    "env2" : {
        ...
        "blacklist" : {
              "files" : ["widgets/file3"],
              "directories" : ["pages", "registration"]
        }
    }
}

@davidlinse
Copy link
Contributor

i'd vote for exclude or ignore as property name and would prefer the first.
my two cents

~david

@beatfactor
Copy link
Member

yeah, I think exclude would be a better name.

@ozymandias547
Copy link
Author

Yea, 'exclude' does sound like a better name for it... 'blacklist' is rather vague come to think of it.

Concerning #107 :
"I think it would be more elegant if we can overwrite the src_folders inside a specific test_settings section. How do you feel about this idea? I'm not sure about the term 'blacklist'... "

Would that require that we have a separate folder for every unique set of tests on each environment? We have a large amount of tests on a large amount of environments, and having individual src_folders might get really big. Also, that would break the DRY principle many times. What do you think?

@ozymandias547
Copy link
Author

Changed to exclude:

"test_settings" : {
    "default" : {
        ...
        "exclude" : {
              "files" : ["widgets/file1"],
              "directories" : ["pages", "templates"]
        },
    "env2" : {
        ...
        "exclude" : {
              "files" : ["widgets/file3"],
              "directories" : ["pages", "registration"]
        }
    },
    "env3" : {
        ...
        exclude : "all"
}

If you think this is a good idea, could you take a look at line#330 & 331 to see if I'm finding the current test folder correctly?

@beatfactor
Copy link
Member

Ok, maybe the src_folders is not such a great idea, so let's forget about that. I think the exclude is a nice feature to have, however I have still some issues with how it's coded. First of all the if lines are very long and could be probably simplified. Also they miss the curly braces, in fact jshint should complain about that.

Secondly, it would be nice if the files and directories could be combined into one array and be specified using glob expressions, like the --filter option. There's already a module included which simplified this.

What do you think? Sorry if I'm being too painstaking here.

@ozymandias547
Copy link
Author

I'll look into those changes - still new to your codebase and the patterns you like to follow, so I don't mind :)

beatfactor added a commit that referenced this pull request Apr 2, 2014
beatfactor added a commit that referenced this pull request Apr 2, 2014
* 'master' of github.com:beatfactor/nightwatch:
  added command timers to show how long it took a command to run in ms
  fixed a small issue with showing output when disabled
  moved the test into protocol actions
  moved the path resolve into index and a few other small refactoring
  Added exclude tests option based on #110
  The screenshot path in junit must be absolute
  fixed typo in example
  style(jshint): satisfy "W004: 'prop' is already defined."
  style(jshint): satisfy "W033: Missing semicolon"
  style(jshint): satisfy "W109: Strings must use singlequote."
  style(white): fix trailing whitespaces
  Allowing client API to use the keys command from the Selenium protocol.
@beatfactor
Copy link
Member

I've implemented the exclude option in 0.4.9. I did it a bit differently so I couldn't use your pull request, I hope you don't mind too much. Here's how it works:

You can specify an array of folders or patterns to match files, all in the same list:

"exclude" : ["excluded-folder"]

or files:

"exclude" : ["tests/src/files-*", "tests/src/other-files*"]

or combined:

"exclude" : ["tests/src/files-*", "tests/src/exclude-folder"]

@beatfactor beatfactor closed this Apr 2, 2014
@ozymandias547
Copy link
Author

That's great! I don't mind at all that you didn't use my PR. You know your code better than anyone. Thanks again for this feature.

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

Successfully merging this pull request may close these issues.

3 participants