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

Feature/2.0 #106

Merged
merged 47 commits into from
Dec 28, 2016
Merged

Feature/2.0 #106

merged 47 commits into from
Dec 28, 2016

Conversation

adamgruber
Copy link
Owner

No description provided.

@adamgruber adamgruber merged commit 22f4a30 into develop Dec 28, 2016
@adamgruber adamgruber deleted the feature/2.0 branch December 29, 2016 19:52
wking added a commit to wking/tap-mocha-reporter that referenced this pull request Jun 7, 2018
This is, as far as I know, Mochawesome-specific, which makes it a bit
of a hack.  The Mochawesome side of this is
adamgruber/mochawesome@65e90621 (implement addContext method for
adding report context to tests, 2016-12-20,
adamgruber/mochawesome#106).  Ideally we'd want a way to trigger this
as part of requesting Mochawesome as the reporter, but the addition
seems harmless enough so I'm just including it every time.

In the diag.source case, I'm just assuming that was already doing what
it was supposed to be doing, so this injection only affects cases
where diag is set but diag.source is not.
wking added a commit to wking/tap-mocha-reporter that referenced this pull request Jun 8, 2018
This is, as far as I know, Mochawesome-specific, which makes it a bit
of a hack.  The Mochawesome side of this is
adamgruber/mochawesome@65e90621 (implement addContext method for
adding report context to tests, 2016-12-20,
adamgruber/mochawesome#106).  Ideally we'd want a way to trigger this
as part of requesting Mochawesome as the reporter, but the addition
seems harmless enough so I'm just including it every time.

In the diag.source case, we might want to expose diagnostics besides
.fn.  But we surely don't want to expose the test source via both .fn
and .context.  For this commit, I've left the diag.source case alone,
but in future work we might want something closer to :

  if (result.diag) {
    var context = Object.assign({}, result.diag)
    if (context.source) {
      var source = context.source
      delete context.source
      this.fn = {
        toString: function () {
          return 'function(){' + source + '\n}'
        }
      }
    }
    if (Object.keys(context).length) {
      this.context = {
        title: 'diagnostic',
        value: context,
      }
    }
  }

I haven't done that here, because Object.assign is not supported on
Internet Exporer, Android webview, or Opera for Android [1], despite
being part of ECMAScript 2015 [2].  Object.keys seems to be supported
everywhere [3].  We may only care about Node for this package, but I'm
being conservative for this commit.

[1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Browser_compatibility
[2]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Specifications
[3]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/keys#Browser_compatibility
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.

1 participant