jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
43.9k stars 6.4k forks source link

Significant difference between behavior of development PC and CI #11010

Open StoneCypher opened 3 years ago

StoneCypher commented 3 years ago

🐛 Bug Report

Hi. I took the plunge and converted a set of 2500 tests from ava to jest. I've been using jest exclusively on new projects for a couple years, but conversions are a bear, so I've been dragging my heels.

I finally got around to it two days ago. Yesterday, I finished. There were two coverage defects - one line that's obviously being covered is marked not-covered, and some blank lines are marked not-covered - which is concerning.

But I figured whatever, I need to get out of ava, let's just go ahead. Pushed it to Github.

I hit a problem I've never seen before. Many of the test specs are behaving as if they're part of the same file.

It's not actually clear to me how this is possible.

 FAIL  src/ts/tests/machine_attributes.spec.ts
  ● Test suite failed to run

    src/ts/tests/machine_attributes.spec.ts:2:7 - error TS2451: Cannot redeclare block-scoped variable 'jssm'.

    2 const jssm = require('../jssm'),
            ~~~~

      src/ts/tests/general.spec.ts:4:7
        4 const jssm = require('../jssm');
                ~~~~
        'jssm' was also declared here.

It appears to be saying that a declaration in one file is conflicting with an unrelated declaration in a different file.

The specs do not load one another (one tiny exception for constants.spec), and are not attached in any meaningful way.

I cannot figure out why a const in one file is conflicting with a const in another file.

Weirder yet? It's not all of them. Almost every spec in this test suite has const jssm = require('../jssm') at the top, but only about half of them collide. The pattern isn't clear.

These should, honestly, be imports; this is the result of my having done this half-awake, converting away from an ava test set that tested the post-compile, instead of the pre-compile. And I'll be moving them to requires next, because these tests are gaining value from TS enforcement

In the meantime, though, my development machine and CI behave radically differently with the same test setup, and that seems bizarre and unsafe.

I will update this issue once I've moved everything to import instead of require. However, this defies the language spec under require, so I think it's a bug even if doing things the normal way "fixes" it

.

To Reproduce

Steps to reproduce the behavior:

.

Expected behavior

I expect a test suite to pass with perfect coverage.

On my dev machine, a test suite passes with near-perfect coverage.

image

Those coverage failures are incorrect, and I don't know what to make of them.

Line 795 is tested here

Lines 928-944 include a lot of non-testable stuff, and are fully tested (1, 2)

.

Link to repl or repo (highly encouraged)

Repo

Continuous integration build showing build problem (look at this instead)

.

envinfo

Works as expected at home, with minor coverage defects. Win10 pro 64, node 14.4.0, nothing installed globally. Typescript version comes from package (currently 4.1.3).

Fails miserably the same way under Github Actions on continuous integration. Tested configurations include:

john@DESKTOP-26H4050 MINGW64 ~/projects/jssm (MoveFromAvaToJest)
$ npx envinfo --preset jest
npx: installed 1 in 2.435s

  System:
    OS: Windows 10 10.0.19041
    CPU: (8) x64 Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz
  Binaries:
    Node: 14.4.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.21.1 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.5 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    jest: ^26.6.3 => 26.6.3
StoneCypher commented 3 years ago

It seems like moving them from require to import resolves whatever's going on

It's not clear to me how that's possible either tbh.

Moving is slow, because in switching from ava testing of js post-compiles to jest testing of ts, I'm finding a lot of sloppy type bugs I should've fixed years ago, lol

StoneCypher commented 3 years ago

oh wow. as i've been working on them, progressively, now one is firing on my dev machine.

so this is probably a cache issue, or if not, a race condition.

i have run npx jest --no-cache -c relevant_config with no impact

image

StoneCypher commented 3 years ago

I don't know what to think now. At first I thought it was a caching issue, where the error was being hidden by a faulty cache of old code and the CI was exposing it because there was never a cache.

So I performed the rain dance fix on weighted_rand_select by changing it from require to import. Gone. Okay, cool.

For fun, change it back. ... nnnnnnnnnnnnnnnnnnnnnnnnnno problem, given the old faulting code. In fact, it seems to be fine either way (as one might expect) now.

Except, it's broken in CI, where there's no prior cache?

And the behavior seems well controlled, so I don't think it's a timing thing.

StoneCypher commented 3 years ago

okay so, in retrospect i've learned the difference between red as total misses and yellow as branch misses, found the html reporter that lets me find then, and so on.

the difference between require and import causing name collisions across unrelated modules remains unclear to me however

Axedyson commented 2 years ago

Except, it's broken in CI, where there's no prior cache?

I've got a similar issue. Since GitHub deletes all caching after 7 days, Jest will fail after 7 days of no CI runs because it can't handle no available cache when running in CI apparently

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

StoneCypher commented 1 year ago

bump :/

github-actions[bot] commented 1 month ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

StoneCypher commented 1 month ago

Bump