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

Get bun test working locally #2204

Merged
merged 8 commits into from
Aug 15, 2024

Conversation

thegedge
Copy link
Collaborator

@thegedge thegedge commented Aug 4, 2024

What does this PR do and why?

Before this PR, bun test would have a ton of failures if you ran it locally. We didn't see this in CI because our runners would test each file separately from one another.

After doing some digging, I found a couple of issues:

  1. AFAICT, bun test maintains globals across modules (unlike jest), so you have to be careful to reset it.
  2. Sometimes tests would hang because failures wouldn't call the done callback.
  3. Some other issue that I could not explain with this test:
    test("it should show friendly message", () => {
    const UserModel = types
    .model("UserModel", {
    id: types.identifier,
    name: types.string
    })
    .views((user) => ({
    get name() {
    return user.name
    }
    }))
    expect(() =>
    UserModel.create({
    id: "chakri",
    name: "Subramanya Chakravarthy"
    })
    ).toThrow("[mobx-state-tree] name property is declared twice")
    })

And the various solutions:

  1. Introducing a setup script that ensures we reset state before/after each test.
  2. Switch to await / async vs callbacks.
  3. Check for overlapping view/properties when instantiating the views vs discovering this issue when getting the atoms, which we were doing here:
    try {
    // TODO: FIXME, make sure the observable ref is used!
    const atom = getAtom(node.storedValue, name)
    ;(atom as any).reportObserved()
    } catch (e) {
    throw new MstError(`${name} property is declared twice`)
    }
    res[name] = this.getChildNode(node, name).snapshot

Steps to validate locally

All of the following should succeed:

  • bun test
  • bun run test:dev
  • bun run test:prod

@coolsoftwaretyler
Copy link
Collaborator

Ooh very exciting! Thanks for figuring this out! Will take a look soon.

@coolsoftwaretyler coolsoftwaretyler merged commit 626262a into mobxjs:master Aug 15, 2024
1 check passed
@thegedge thegedge deleted the make-bun-test-work branch November 9, 2024 16:08
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