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

Recent bug fixes #55

Merged
merged 3 commits into from
Dec 12, 2018
Merged

Recent bug fixes #55

merged 3 commits into from
Dec 12, 2018

Conversation

grawk
Copy link
Member

@grawk grawk commented Dec 10, 2018

@dwbruhn
Copy link
Collaborator

dwbruhn commented Dec 10, 2018

Awesome! Testing this out...

@grawk
Copy link
Member Author

grawk commented Dec 10, 2018

Thanks Dan. Let me know how it goes. Feel free to code review this PR as well 😄

@grawk grawk requested a review from dwbruhn December 10, 2018 17:44
@dwbruhn
Copy link
Collaborator

dwbruhn commented Dec 10, 2018

Okay, regarding #32, just want to make sure I understand. Is the expectation that the whole base data will be merged into every parallel run of a non-base profile? Here's what I have in nemo.config.js. The base profile has all the data, while the chrome profile has none of its own.

profiles: {
        base: {
            maxConcurrent: 1,
            tests: path.resolve("test/nemo2", "*test.js"),
            driver: {
                browser: "chrome"
            },
            mocha: {
                timeout: 180000,
                reporter: "mochawesome"
            },
            parallel: "data",
            data: {
                A: {
                    case: "A",
                    B: {
                        THIS_KEY_SHOULD_HAVE_NO_SIBLINGS_WHEN_UNDER_B: 1
                    }
                },
                B: {
                    case: "B"
                }
            }
        },
        chrome: {
            driver: {
                browser: "chrome"
            }
        }
    }

The test code:

describe('@firstTest@', function () {
  it('should load a website', async function () {
    console.log("Running case", this.nemo.data.case);
    console.log("Data is:");
    console.log(JSON.stringify(this.nemo.data, null, 2));
  });
});

When running the base profile itself, the data is clean:

  @firstTest@
Running case A
Data is:
{
  "B": {
    "THIS_KEY_SHOULD_HAVE_NO_SIBLINGS_WHEN_UNDER_B": 1
  },
  "case": "A"
}
  @firstTest@
Running case B
Data is:
{
  "case": "B"
}

But with the chrome profile, the whole base data gets merged into every instance:

  @firstTest@
Running case A
Data is:
{
  "A": {
    "B": {
      "THIS_KEY_SHOULD_HAVE_NO_SIBLINGS_WHEN_UNDER_B": 1
    },
    "case": "A"
  },
  "B": {
    "THIS_KEY_SHOULD_HAVE_NO_SIBLINGS_WHEN_UNDER_B": 1,
    "case": "B"
  },
  "case": "A"
}
  @firstTest@
Running case B
Data is:
{
  "A": {
    "B": {
      "THIS_KEY_SHOULD_HAVE_NO_SIBLINGS_WHEN_UNDER_B": 1
    },
    "case": "A"
  },
  "B": {
    "case": "B"
  },
  "case": "B"
}

It's fine if that's expected. Just want to make sure I'm clear!

@grawk
Copy link
Member Author

grawk commented Dec 10, 2018

Yes, that's the expectation.. I think it's fair to ask for clear documentation on the data merge before accepting this PR :)

@dwbruhn
Copy link
Collaborator

dwbruhn commented Dec 10, 2018

Thanks! I was able to verify that this fixes all 3 issues.

@grawk
Copy link
Member Author

grawk commented Dec 10, 2018

Thanks @dwbruhn .. I'll work up a section on data config override behavior into the README .. Will ask that you take one last look at that tomorrow before merge/publish.

@dwbruhn
Copy link
Collaborator

dwbruhn commented Dec 11, 2018

@grawk Sure, just let me know! Looking forward to these fixes 😀

@grawk grawk merged commit e070aca into master Dec 12, 2018
@grawk grawk deleted the fix.jsconfig.plugins.data branch December 12, 2018 17: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.

2 participants