oven-sh/bun

SpyOn doesn't work on ESM namespace object properties

Closed this issue · 4 comments

coratgerl commented

What version of Bun is running?

1.0.0

What platform is your computer?

Darwin 22.5.0 arm64 arm

What steps can reproduce the bug?

The spyOn of bun test doesn't work when the function is import from a different file of the test file. Like the example below:

Test file :

import { describe, expect, it, spyOn } from 'bun:test'
import * as fileA from './fileA'

describe('fileB', () => {
	it('should spy the say toto in file A', async () => {
		const spySayToto = spyOn(fileA, 'sayToto')

		fileA.sayToto()

                // This test failed and have 0 as value
		expect(spySayToto).toHaveBeenCalledTimes(1)
	})
})

File A with function to spy:

export const sayToto = () => {
	console.log('toto')
}

What is the expected behavior?

The function have been called 1 times and not zero.

What do you see instead?

No response

Additional information

No response

winghouchan commented

I've found a workaround by assigning the imported function to an object created in the test file:

index.ts

export function fn() { ... }

index.test.ts

import { expect, spyOn, test } from "bun:test";
import { fn } from "./index";

test("spied fn is called", () => {
  const obj = { fn };
  const spy = spyOn(obj, "fn");

  obj.fn();

  expect(spy).toHaveBeenCalledTimes(1);
});
winghouchan commented

I've taken a few steps to look at this but I'm not familiar with C++ so I probably won't get anywhere. Documenting what I find anyway in case it'll be useful. So far:

  1. Created a test case for this scenario (d1d80da). The case itself is just a copy of this test case but just uses a function imported from a fixture file. It fails as expected.
  2. Suspect the problem is somewhere here:

    bun/src/bun.js/bindings/JSMockFunction.cpp

    Lines 570 to 659 in 687e31d

    extern "C" EncodedJSValue JSMock__jsSpyOn(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callframe)
    {
    auto& vm = lexicalGlobalObject->vm();
    auto scope = DECLARE_THROW_SCOPE(vm);
    auto* globalObject = jsDynamicCast<Zig::GlobalObject*>(lexicalGlobalObject);
    if (UNLIKELY(!globalObject)) {
    throwVMError(globalObject, scope, "Cannot run spyOn from a different global context"_s);
    return {};
    }
    JSValue objectValue = callframe->argument(0);
    JSValue propertyKeyValue = callframe->argument(1);
    if (callframe->argumentCount() < 2 || !objectValue.isObject()) {
    throwVMError(globalObject, scope, "spyOn(target, prop) expects a target object and a property key"_s);
    return {};
    }
    PropertyName propertyKey = propertyKeyValue.toPropertyKey(globalObject);
    RETURN_IF_EXCEPTION(scope, {});
    if (propertyKey.isNull()) {
    throwVMError(globalObject, scope, "spyOn(target, prop) expects a property key"_s);
    return {};
    }
    JSC::JSObject* object = objectValue.getObject();
    if (object->type() == JSC::JSType::GlobalProxyType)
    object = jsCast<JSC::JSGlobalProxy*>(object)->target();
    JSC::PropertySlot slot(object, JSC::PropertySlot::InternalMethodType::HasProperty);
    bool hasValue = object->getPropertySlot(globalObject, propertyKey, slot);
    // easymode: regular property or missing property
    if (!hasValue || slot.isValue()) {
    JSValue value = jsUndefined();
    if (hasValue) {
    value = slot.getValue(globalObject, propertyKey);
    if (jsDynamicCast<JSMockFunction*>(value)) {
    return JSValue::encode(value);
    }
    }
    auto* mock = JSMockFunction::create(vm, globalObject, globalObject->mockModule.mockFunctionStructure.getInitializedOnMainThread(globalObject), CallbackKind::GetterSetter);
    mock->spyTarget = JSC::Weak<JSObject>(object, &weakValueHandleOwner(), nullptr);
    mock->spyIdentifier = propertyKey.isSymbol() ? Identifier::fromUid(vm, propertyKey.uid()) : Identifier::fromString(vm, propertyKey.publicName());
    mock->spyAttributes = hasValue ? slot.attributes() : 0;
    unsigned attributes = 0;
    if (hasValue && ((slot.attributes() & PropertyAttribute::Function) != 0 || (value.isCell() && value.isCallable()))) {
    if (hasValue)
    attributes = slot.attributes();
    mock->copyNameAndLength(vm, globalObject, value);
    attributes |= PropertyAttribute::Function;
    object->putDirect(vm, propertyKey, mock, attributes);
    RETURN_IF_EXCEPTION(scope, {});
    pushImpl(mock, globalObject, JSMockImplementation::Kind::Call, value);
    } else {
    if (hasValue)
    attributes = slot.attributes();
    attributes |= PropertyAttribute::Accessor;
    object->putDirect(vm, propertyKey, JSC::GetterSetter::create(vm, globalObject, mock, mock), attributes);
    // mock->setName(propertyKey.publicName());
    RETURN_IF_EXCEPTION(scope, {});
    pushImpl(mock, globalObject, JSMockImplementation::Kind::ReturnValue, value);
    }
    mock->spyOriginal.set(vm, mock, value);
    if (!globalObject->mockModule.activeSpies) {
    ActiveSpySet* activeSpies = ActiveSpySet::create(vm, globalObject->mockModule.activeSpySetStructure.getInitializedOnMainThread(globalObject));
    globalObject->mockModule.activeSpies.set(vm, activeSpies);
    }
    ActiveSpySet* activeSpies = jsCast<ActiveSpySet*>(globalObject->mockModule.activeSpies.get());
    activeSpies->add(vm, mock, mock);
    return JSValue::encode(mock);
    }
    // hardmode: accessor property
    throwVMError(globalObject, scope, "spyOn(target, prop) does not support accessor properties yet"_s);
    return {};
    }
coratgerl commented

Yes I also look the issue few days ago, I think the problem is that the function is not in the globalContext because it's is in a different file. We need to find the right globalContext I think but I am not an expert on JavascriptCore so I am not sure about this.

Jarred-Sumner commented

This happens because the ES Module namespace object symbol table needs to be mutated to update the binding for the ESM export to the new wrapped value

  • 👍2