There probably shouldn't be a test.only() used at /test/unit/io/files.js line 469

[akshay-99]

So I was going through the code for the tests and felt that this was important to bring to the attention of the maintainers.
There probably shouldn't be a test.only() used here

I don't fully understand the tests yet so I am sorry if this ends up being stupid.

[welcome]

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

[stalgiag]

Thanks for finding that @akshay-99 . Luckily I think that since it is the only test in that suite this doesn't cause any other tests to not run. But we should still remove it in case any future tests go in this suite.

[akshay-99]

Unfortunately, that doesn't seem to be the case. When I remove .only() and try npm test on the current code on master, it runs a whole lot more test cases that didn't run before.
2 of them fail
This is in the "mochaChrome:test" (mochaChrome) task

1378 passing (14s)
 10 pending
 2 failing
 1) Error Helpers
      friendly error logger
        basic:
    AssertionError: got unwanted exception: expected [Function] to not throw 'Error' but 'TypeError: Cannot read property \'substring\' of undefined' was thrown
     at Function.assert.doesNotThrow (http://localhost:9001/node_modules/chai/chai.js:3300:35)
     at Context.<anonymous> (unit/core/error_helpers.js:19:14)

 2) DOM
      p5.Element.prototype.position
        time and date
          p5.prototype.millis
            result should be greater than running time:
    AssertionError: everything is ok: expected 49.689999999827705 to be > 50
 

Warning: [object Object] Use --force to continue.

Aborted due to warnings.
npm ERR! Test failed.  See above for more details.

@stalgiag @outofambit can you please confirm ?

This is the output with the .only() in place, as is in master

Running "mochaChrome:test" (mochaChrome) task

  DOM
    p5.Element.prototype.position
      Files
        p5.prototype.saveStrings
          ✓ should be a function
  1 passing (9ms)

  DOM
    p5.Element.prototype.position
      Files
        p5.prototype.saveStrings
          ✓ should be a function
  1 passing (8ms)

Running "mochaTest:test" (mochaTest) task
 8   -_-_-_-_-_,------,
 0   -_-_-_-_-_|   /\_/\ 
 0   -_-_-_-_-^|__( ^ .^) 
     -_-_-_-_-  ""  "" 

  8 passing (16ms)

This also seems to be the output of the most recent run of the CI.
Is it only running "should be a function" in the "mochaChrome:test" (mochaChrome) task?

[stalgiag]

Hm I thought the .only only applies in the suite scope but it appears to have different behavior. It looks like the .only was added about two months ago.

I can't look right now but I think the removal of the .only should happen along with fixing the broken tests and/or tested behavior.

[akshay-99]

The first test failure was due to not passing a method argument to _friendlyError in the unit test itself. I am trying to work out second one.

[akshay-99]

The second test failure appears to be due to a precision error. Date.now() gives time to the nearest millisecond while window.performance.now() used in millis() gives it to 0.000000001 milliseconds.
Every time, the test crashes due to an error < 0.5 ms. This was due to a change introduced in #4286 .
Perhaps we should replace the Date.now() in test to window.performace.now()as well to match precision?
@stalgiag @outofambit @limzykenneth

[limzykenneth]

@akshay-99 Is the two values being compared against each other? I'm not sure if changing it to window.performance.now() is going to make the comparison the same as it is made to avoid giving very precise measurement as a counter measure against Meltdown vulnerability.

You can still try though. If it doesn't work just round it up to the nearest millisecond which should be enough for our purpose.

[akshay-99]

This did solve the failure of result should be greater than running time
I am not getting why the test cases are running twice.
For example in the same run of npm test I got

helpForMisusedAtTopLevelCode
      ✓ help for constants is shown
      ✓ help for functions is shown
      ✓ help for variables is shown

and also

    helpForMisusedAtTopLevelCode
p5.js translator called before translations were loaded
      1) help for constants is shown
p5.js translator called before translations were loaded
      2) help for functions is shown
p5.js translator called before translations were loaded
      3) help for variables is shown

I checked a commit from early January and there too the test cases under the mochaChrome:test" (mochaChrome) task were running twice
For example, in the same run

  1363 passing (15s)
  10 pending

appears here and also appears here, after running the same bunch of tests again

After solving result should be greater than running time,
3 testcases of helpForMisusedAtTopLevelCode mentioned above failed in the second run but passed in the first

[akshay-99]

Okay it took me an embarrassingly long time to figure out that the second run of the unit tests was for the minified library. And now it makes sense that the 3 test cases fail only for the minified library as the translator is not initialised there.
@outofambit should I add an if condition helpForMisusedAtTopLevelCode to use the translator only if IS_MINIFIED is not set? This will correct these 3 test failures