tacoss / grunt-nightwatch

Run your Nightwatch.js tests with Grunt
50 stars 22 forks source link

Custom settings path: json file is not interpreted correctly #33

Closed tortila closed 9 years ago

tortila commented 9 years ago

Hi, so this issue relates to #30. Starting from v0.4.1 grunt-nightwatch supports custom json object with settings for nightwatch, but these settings are not read correctly. Let me give you an example:

In options for grunt task I have:

    var selenium_version = "2.42.2";
    var path = require("path");
    options: {
        standalone: true,
        jar_path: path.resolve(__dirname + "/../tests/functional/lib/selenium-server-standalone-" + selenium_version + ".jar"),
        jar_version: selenium_version,
        config_path: path.resolve(__dirname + "/../tests/functional/nightwatch.json")
    }

And in nightwatch.json specified by config_path, among other things, I specify test_settings:

    "test_settings": {
        "default": {
            "launch_url": "",
            "selenium_port": 4444,
            "selenium_host": "localhost",
            "silent": true,
            "desiredCapabilities": {
                "browserName": "chrome",
                "javascriptEnabled": true,
                "acceptSslCerts": true
            }
        }

and, in the same file, some basic config:

    "src_folders": ["spec"],
    "output_folder": "../../test-reports",
    "custom_commands_path": "commands",
    "custom_assertions_path": "assertions",
    "globals_path": "globals.json",

Now, by running this task I have the following output (well, a piece of it, the rest is not necessary):

Running "nightwatch" task
Reading <absolute_path>/tests/functional/nightwatch.json...OK
Parsing <absolute_path>/tests/functional/nightwatch.json...OK
>> Custom JSON-file: <absolute_path>/tests/functional/nightwatch.json
>> Task options
{"standalone":true,"jar_path":"<absolute_path>/tests/functional/lib/selenium-server-standalone-2.42.2.jar",
"jar_url":"http://selenium-release.storage.googleapis.com/{x}.{y}/selenium-server-standalone-{x}.{y}.{z}.jar",
"jar_version":"2.42.2","config_path":"<absolute_path>/tests/functional/nightwatch.json"}
>> Using Selenium from <absolute_path>/tests/functional/lib/selenium-server-standalone-2.42.2.jar
Starting selenium server... started - PID:  <selenium_pid>

[Sorted] Test Suite
===================

Running:  assertion 

INFO Request: POST /wd/hub/session 
 - data:  {"desiredCapabilities":{"browserName":"firefox","javascriptEnabled":true,"acceptSslCerts":true,
"platform":"ANY","name":"Sorted"}} 
 - headers:  {"Content-Type":"application/json; charset=utf-8","Content-Length":129}

And here are 2 things that bother me:

  1. desiredCapabilities has now firefox as default browser, and not chrome as I specified in my custom settings file
  2. the first test that is being run (or at least started) is actually a custom assertion, that is being started only because it's the first folder (alphabetically) in the same directory as my custom settings file - even though I specified src_folders.

So my conclusion is that the settings are not read/interpreted correctly. Could you please take a look at this problem?

pateketrueke commented 9 years ago

I'll take a look to that, in fact, have you tried Nightwatch standalone for running your tests before?

tortila commented 9 years ago

Of course I did, tests run smoothly with the same config file as (partially) shown above (fyi, nightwatch installed globally, but it doesn't change much, does it?).

pateketrueke commented 9 years ago

I got it.

It seems my merge-strategy is not fine tuned.

Thanks for your feedback, i'll publish a patch for this soon.

Edit: BTW, I've published to the develop branch. Could you run your tests against?

tortila commented 9 years ago
>> Failed to load external global file: 
External global file could not be located - using globals.json.

when running the task using develop branch of grunt-nightwatch. Before that it just prints all the settings picked from my custom config file. And prompts reading and parsing the config file - twice. \ Edit: it's worth mentioning that I don't have any external global file declared in globals.json (globals_path in custom config file points to globals.json, but no external globals exist).

>> No tests defined! using source folder: 
<absolute_path>/spec

while the path should be: <absolute_path>/tests/functional/spec. So the paths provided in my custom config file (listed in my previous comments) are not resolved correctly, but rather they are resolved as relative to the location of gruntfile, and not as relative to the location of config file where they are specified.

pateketrueke commented 9 years ago

So the paths provided in my custom config file (listed in my previous comments) are not resolved correctly, but rather they are resolved as relative to the location of gruntfile, and not as relative to the location of config file where they are specified.

IMHO, Nighwatch is responsible for anything what happens with its configuration.

The provided config_path option is only a shortcut for loading options/settings from another file rather than nightwatch.json or Gruntfile within project's directory.

Of course you cannot use relative paths from the config-files's location, always will be relative from the working directory, which for all grunt tasks stands where the Gruntfile resides.

BTW; I've published some changes to the developbranch, could you try again?

tortila commented 9 years ago

BTW; I've published some changes to the developbranch, could you try again?

Still the same.

Of course you cannot use relative paths from the config-files's location, always will be relative from the working directory

In my opinion it's doable via adding path resolving (concatenating path.relative between config_path directory and Gruntfile's directory in front of paths specified in config_path).

tortila commented 9 years ago

FYI, I managed to fix the issue of relative paths, so let me know if you want to accept a pull request for it.

Then there's another issue that I mentioned above:

When I delete the property globals_path from my custom config file, there is also an issue of having both: "start_process": true in config file and "standalone": true in task's options.

That should be fixed, but I'm not sure if it's the issue of merging the settings or the issue of nightwatch itself, because when I try to run it (globally installed) with --env default,default I get exactly the same error, because Selenium is started twice. Any thoughts?

tortila commented 9 years ago

My solution still doesn't fix the selenium issue mentioned above, nor resolving paths which keys are inside some other objects in custom json, but fixes the path resolving to globals, asseertions, commands and output - so basically fixes this issue

pateketrueke commented 9 years ago

because when I try to run it (globally installed) with --env default,default I get exactly the same error, because Selenium is started twice. Any thoughts?

cc @beatfactor

tortila commented 9 years ago

I tried running my tests on your latest commit and it works just fine, I didn't find any bugs. I think it's worth to merge the changes into master.

I'm still working on this selenium issue, but I'll open another issue for that.

tortila commented 9 years ago

btw, paths mapped under keys inside some other objects in custom config json, like:

"test_settings": {
    "default": {
        "screenshots": {
            "path": "/some/path/to/screenshots/directory"
        }
    }
}

are yet to be fixed. I tried to modify them, but could not access the values like a normal json object - any ideas on how to approach this?

pateketrueke commented 9 years ago

but could not access the values like a normal json object

I'm not understand yet... what do you mean?

tortila commented 9 years ago

In the example above, normally you would access the value of path that is inside of screenshots by screenshots["path"] or screenshots.path. In your code there's screenshots as one of the settings, but how can I access its path, to resolve it as other paths from custom config file are resolved?

pateketrueke commented 9 years ago

I'm not sure if you're missing something:

if (_.isObject(settings.screenshots)) {
  settings.screenshots.path = $.resolvePath(options.config_path, settings.screenshots.path);
}

The same applies for each target I guess but certainly I haven't clear that.

tortila commented 9 years ago

IMO it would be better to keep all paths in one place and iterate over them instead of checking and listing all the scenarios (check screenshots.path, check all webdrivers' paths, check selenium log and server directories, ...). Apart from that I tried your approach and still cannot get it working.

As I wrote above, I tried to access such paths and put them together with others in paths array, but with no success - it seems that those nested values cannot be accessed the way all paths are accessed now.

Could you try your approach on some example and maybe introduce it in develop?

pateketrueke commented 9 years ago

it seems that those nested values cannot be accessed the way all paths are accessed now.

I think your approach is fine but you should do it recursively, the key in settings.path for the directory is path, so keeping it along with the other options just would work as you did?

Could you try your approach on some example and maybe introduce it in develop?

I've merged your changes into master and patch your solution.

Please review and test, I hope this fulfill your needs.

tortila commented 9 years ago

That fix kind of destroyed what was already working.

Right now (tried master branch) the path to screenshots is resolved properly, but additionally src_folders is not resolved properly (and it was working on develop before).

tortila commented 9 years ago

Opened another merge request, that should solve this issue.