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

ci: ensure all tests are performed #2423

Merged
merged 5 commits into from
Jan 31, 2024
Merged

Conversation

wellwelwel
Copy link
Collaborator

@wellwelwel wellwelwel commented Jan 31, 2024

While working on another PR, I noticed a problem with tests that have been occurring for a long time.
Tests are started using urun, but when analyzing the logs, there are some inconsistencies:

1.

It counts the total files, but change the step while still running the previous test:

[0:00:00 0 1/132 0.8% node test/integration/test-auth-switch-plugin-async-error.js]
[0:00:00 0 2/132 1.5% node test/integration/test-auth-switch-plugin-async-error.js]

2.

Some time counters don't respect a specific rule:

[0:00:00 0 4/132 3.0% node test/integration/test-auth-switch.js]
[0:00:00 0 4/132 3.0% node test/integration/test-handshake-unknown-packet-error.js]
[0:00:00 0 5/132 3.8% node test/integration/test-handshake-unknown-packet-error.js]
[0:00:00 0 5/132 3.8% node test/integration/test-namedPlaceholders.js]

3.

The total of current valid tests are not 132, but 133 (18 for unit and 115 for integration).


4.

Some tests aren't performed, giving the false impression that all the tests have passed.
The missing tests are random. For example, sometimes test-packet-parser.js is missing, other times it is there.


5.

In contrast, the counter works fine locally, but when running tests locally, the missing test range is more variable:

Local tests in question are running on a Mac (arm64).

  • Instead of running 18 unit tests, it runs ~8
  • Instead of running 115 integration tests, it runs ~18

6.

When running tests on GitHub Actions:

  • ~38 (of 18) unit tests are displayed, but if we disregard the repeated ones, usually it runs all 18 unit tests.
  • ~144 (of 115) integration tests are displayed, but if we consider that each appears between two or three times, it's possible that less than ~70-80 from 115 integration tests are running.
  • For my next PR, I created an additional test directory inside the "unit" and both locally and on GitHub Actions, it was ignored, while the coverage shows it as covered by tests (that may not be performed).

7.

In the case of unhandledRejection or uncaughtException, the logs are displayed, but the process exit is 0.


This PR

  • This PR implements a simple native solution to list all the contents from test directories, listing and running all current 133 valid tests (js, mjs) sequentially.
  • It will also ensure that unhandledRejection and uncaughtException are finished with process 1.
  • This PR also keeps the environment variable FILTER in effect.
  • Fix a false positive from CodeQL about Regular expression injection when using FILTER on tests
  • It doesn't consider the builtin-runner tests, as they already have their own test.

Example

You can also see this PR Actions logs.

I kept the logs to be as similar as possible from urun:

Directory: test/unit

✅ [ 0 00:00:00  1/18   6% node test/unit/commands/test-query.js ]

✅ [ 0 00:00:00  2/18  11% node test/unit/commands/test-quit.js ]

✅ [ 0 00:00:00  3/18  17% node test/unit/connection/test-connection_config.js ]

# ...
Directory: test/integration

test connect timeout
ok
✅ [ 0 00:00:02   1/115   1% node test/integration/config/test-connect-timeout.js ]

✅ [ 0 00:00:03   2/115   2% node test/integration/config/test-typecast-global-option.js ]

✅ [ 0 00:00:03   3/115   3% node test/integration/connection/encoding/test-charset-results.js ]

Error example:

Failed: Pool exposes escape

  AssertionError [ERR_ASSERTION]: '123' == '1234'
      at exposes escape (/usr/app/test/unit/test-Pool.js:12:12)
      at TestCase.run (/usr/app/node_modules/utest/lib/TestCase.js:30:10)
      at Collection._runTestCase (/usr/app/node_modules/utest/lib/Collection.js:44:6)
      at Collection.run (/usr/app/node_modules/utest/lib/Collection.js:23:10)
      at process.processTicksAndRejections (node:internal/process/task_queues:77:11)

1 fail | 8 pass | 1 ms 
❌ [ 1 00:00:02 17/18  94% node test/unit/test-Pool.js ]

Naturally: About to exit with code: 1.


Notes

  • The Action in question is CI - Linux #1017, but everyone will serve as an example.

@wellwelwel wellwelwel merged commit 7cc9064 into sidorares:master Jan 31, 2024
52 checks passed
@wellwelwel wellwelwel deleted the tests-issue branch January 31, 2024 22:26
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.

1 participant