publiclab / infragram

A minimal core of the Infragram.org project in JavaScript
https://infragram.org/sandbox/
GNU General Public License v2.0
45 stars 166 forks source link

Jest.js Configurations #449

Closed Forchapeatl closed 2 years ago

Forchapeatl commented 2 years ago

Jest.js Configurations

Fixes #418 and #448

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

gitpod-io[bot] commented 2 years ago

jywarren commented 2 years ago

Hi @Forchapeatl this is looking great. Before we merge, let's try to add just one test, of something really simple or obvious. Then I can help connect it with GitHub actions and we'll see the green check mark. How does that sound?

Forchapeatl commented 2 years ago

image

Hello @jywarren . I have done as requested. Is the /test/ui/ directory okay to work with ? Please let me know if it needs any adjustments.

jywarren commented 2 years ago

Hi, it looks great! Just added the command to package.json so it runs in GitHub Actions now!

https://github.com/publiclab/infragram/blob/a69271b01e3c675bd9daba3016c019fce1915510/.github/workflows/tests.yml#L25

But now it says:

> infragram@0.3.0 test /home/runner/work/infragram/infragram
> jest

sh: 1: jest: not found
npm ERR! Test failed.  See above for more details.
Error: Process completed with exit code 1.

So I think we may need to add jest as a dependency. Yes, look:

https://github.com/publiclab/PublicLab.Editor/blob/main/package.json#L65-L66

jywarren commented 2 years ago

Hmm, ok, it looks like it's running now, but it got stuck on this line:

https://github.com/publiclab/infragram/runs/8025926511?check_suite_focus=true#step:7:11

jywarren commented 2 years ago

Hi @Forchapeatl just wanted to say i think you're trying everything I would have, good detective work!

One clue is, that error?.stack seems to be ES6 syntax... is that right? Like, are we allowed to use a question mark in a variable name in < ES6? Is it possible we're running things in the wrong node version or something? Does this code run perfectly locally and what node version are you running?

https://github.com/facebook/jest/blame/a20bd2c31e126fc998c2407cfc6c1ecf39ead709/packages/jest-cli/src/cli/index.ts#L40

Here's the line that's failing. Ah - it's this operator - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Let's check version in the GitHub Actions environment. We are in Node 10 here, should we upgrade it?

https://github.com/publiclab/infragram/blob/1dabd5ceee543ed02985b1c60b7d841bbf92ac94/.github/workflows/tests.yml#L11

Forchapeatl commented 2 years ago

image Hello @jywarren , I am using node version 14.19.3 on ubuntu 20.4 . Yes please , let's update it.

jywarren commented 2 years ago

I'll try updating node now!

jywarren commented 2 years ago

Hey, it's working now! Jest tests are definitely running!

Run npm test

> infragram@0.3.0 test
> jest

FAIL test/ui/presets.test.js
  Presets Raw 
    ✕ Test values of Presets Raw and related feilds (1 ms)
  Presets Ndvi Blue 
    ✕ Test values of Presets Ndvi Blue and related feilds
  Presets Ndvi Blue color 
    ✕ Test values of Presets Ndvi Blue color and related feilds
  Presets Ndvi Red
    ✕ Test values of Presets Ndvi Red and related feilds
  Presets Ndvi Red color
    ✕ Test values of Presets Ndvi Red color and related feilds

  ● Presets Raw  › Test values of Presets Raw and related feilds

    net::ERR_CONNECTION_REFUSED at http://127.0.0.1:8080/index.html

      1 | const timeout = process.env.SLOWMO ? 30000 : 100000;
      2 | beforeAll(async () => {
    > 3 |   await page.goto('http://127.0.0.1:8080/index.html', {waitUntil: 'domcontentloaded'});
        |   ^
      [4](https://github.com/publiclab/infragram/runs/8074362111?check_suite_focus=true#step:7:5) | });
      [5](https://github.com/publiclab/infragram/runs/8074362111?check_suite_focus=true#step:7:6) |
      [6](https://github.com/publiclab/infragram/runs/8074362111?check_suite_focus=true#step:7:7) | describe('Presets Raw ', () => {

OK, let's see what's up with the port 8080 issue.

jywarren commented 2 years ago

OK, i believe tests are all running fine now -- if you'd like we can comment out the preset tests for now and just merge the test suite itself, then you can keep working on more tests?

FAIL test/ui/presets.test.js
  Presets Raw 
    ✕ Test values of Presets Raw and related feilds (145 ms)
  Presets Ndvi Blue 
    ✕ Test values of Presets Ndvi Blue and related feilds (25 ms)
  Presets Ndvi Blue color 
    ✕ Test values of Presets Ndvi Blue color and related feilds (36 ms)
  Presets Ndvi Red
    ✕ Test values of Presets Ndvi Red and related feilds (31 ms)
  Presets Ndvi Red color
    ✕ Test values of Presets Ndvi Red color and related feilds (31 ms)

  ● Presets Raw  › Test values of Presets Raw and related feilds
jywarren commented 2 years ago

Ah, what did you find? Some of these passed, huh?

Forchapeatl commented 2 years ago

Ah, what did you find? Some of these passed, huh?

Yes , we pass on github actions only when we log the CSS styling https://github.com/publiclab/infragram/blob/a74ce8382687bc965de6064294bb7288cce499b9/test/ui/presets.test.js#L39

I believe it's ready to merge

jywarren commented 2 years ago

Oh... Is it reading the log as the results? By convention, an output of the digit 0 is read as failing, I think? Or is it 1?

You can read how some other folks are configuring jest here and some discussion of exit codes: https://github.com/facebook/jest/issues/9324

But it's a long thread and there are probably better resources for how log statements affect success or failure.

I still think the best approach is to closely match an existing jest test suite, like the image sequencer project. Let me take a look at these logs, but can you ask Cess and Tilda for help in the chat as well?

Thanks for sticking with this. We're almost there.

jywarren commented 2 years ago

That's strange.

FAIL test/ui/presets.test.js
  Presets Raw 
    ✕ Test values of Presets Raw and related feilds (2294 ms)

It doesn't error it actually fails. Meaning it successfully runs the whole test but it reads the test output as a failure. Could this have something to do with the asynchronicity of the test? Like, maybe it just waits until the time out, and then fails. But if we add a comment, somehow, it waits long enough? I don't know if that makes any sense.

Forchapeatl commented 2 years ago

@jywarren ,please may I merge this?

jywarren commented 2 years ago

Was it toEqual vs. toBe? Just let me know by commenting when you solve something so I can follow along! Thanks, yes, let's merge it! You go ahead!

jywarren commented 2 years ago

Great job!!