DataDog / dd-trace-js

JavaScript APM Tracer

https://docs.datadoghq.com/tracing/

Other

647 stars 306 forks source link

Instrumentation for Mocha >=5.2.0 is broken #2595

Closed jodigiordano closed 1 year ago

jodigiordano commented 1 year ago

Expected behaviour

Mocha tests should run.

Actual behaviour

All Mocha tests are skipped.

Environment

Operation system: GNU/Linux Debian 11
Node.js version: v16.17.1
Tracer version: 3.9.0

The issue

The culprit is this new piece of code:

It overrides the original Mocha.run.
It sends messages (ci:mocha:configuration and ci:mocha:test-suite:skippable) and will run the original Mocha.run upon receiving the responses.
From what I understand, the Mocha plugin should listen to those messages but it doesn't.

juan-fernandez commented 1 year ago

Expected behaviour

Mocha tests should run.

Actual behaviour

All Mocha tests are skipped.

Environment

Operation system: GNU/Linux Debian 11

Node.js version: v16.17.1

Tracer version: 3.9.0

The issue

The culprit is this new piece of code:

It overrides the original Mocha.run.

It sends messages (ci:mocha:configuration and ci:mocha:test-suite:skippable) and will run the original Mocha.run upon receiving the responses.

From what I understand, the Mocha plugin should listen to those messages but it doesn't.

hi @jodigiordano ! Thanks for reporting this issue.

I can't reproduce it 🤔. Could you help me by providing:

a small reproducible case
if 1. is not possible, the dd-trace config you're using to run mocha: e.g. are you using NODE_OPTIONS to init dd-trace?
any mocha non-default config you might be using
any dd-trace error log you see when running mocha

juan-fernandez commented 1 year ago

Expected behaviour Mocha tests should run. Actual behaviour All Mocha tests are skipped. Environment

Operation system: GNU/Linux Debian 11

Node.js version: v16.17.1

Tracer version: 3.9.0

The issue The culprit is this new piece of code:

It overrides the original Mocha.run.

It sends messages (ci:mocha:configuration and ci:mocha:test-suite:skippable) and will run the original Mocha.run upon receiving the responses.

From what I understand, the Mocha plugin should listen to those messages but it doesn't.

hi @jodigiordano ! Thanks for reporting this issue.

I can't reproduce it 🤔. Could you help me by providing:

a small reproducible case

if 1. is not possible, the dd-trace config you're using to run mocha: e.g. are you using NODE_OPTIONS to init dd-trace?

any mocha non-default config you might be using

any dd-trace error log you see when running mocha

I found a bug where if dd-trace could not be initialized correctly the tests are not running at all. I'll be pushing a fix right away, but I want to confirm that this is what's happening to you.

juan-fernandez commented 1 year ago

The fix is in https://github.com/DataDog/dd-trace-js/pull/2599. I'll wait for your feedback before merging, to make sure this was your issue.

jodigiordano commented 1 year ago

Hi @juan-fernandez ! Thank you very much for looking into this. I confirm that:

I am in fact in a situation where dd-trace is not initialized, as we enable/disable dd-trace in our tests via a flag. :scream_cat:
The fix you produced solve my issue! :rocket:

Cheers! :beers:

juan-fernandez commented 1 year ago

2599 is merged. The fix will be in the next release, which will happen tomorrow 😄

juan-fernandez commented 1 year ago

2599 is merged. The fix will be in the next release, which will happen tomorrow 😄

https://github.com/DataDog/dd-trace-js/releases/tag/v3.9.3 and https://github.com/DataDog/dd-trace-js/releases/tag/v2.22.3 should include this fix 😄