todogroup / repolinter

Repolinter, The Open Source Repository Linter
https://todogroup.github.io/repolinter/
Apache License 2.0
428 stars 74 forks source link

Selective test run for git tests #283

Closed JeroenKnoops closed 1 year ago

JeroenKnoops commented 1 year ago

Motivation

Use SinonJs stub for the GitAllCommits function to speed up the tests. The tests were performing the rules on a big repository with a lot of commits, and the timeout was hit very frequently.

Now this should be resolved.

Now the tests will only check the following three commits:

Commit sha test-case
3e66e3ec616d59f813bdb878e1146d03872a096e git_grep_commits
c9e1b59c86c119a5a67389ffd13d026c6058492a git_grep_commits
260f8cc14d6ecf0ff1f0162f88086143d813967a git_list_tree

Proposed Changes

Test Plan

Ran various individual tests locally

./node_modules/mocha/bin/mocha.js tests/rules/git_grep_commits_tests.js

Ran all tests locally

npm test
JeroenKnoops commented 1 year ago

56a09459e55a2671973bdf38176331d75f4b1af8 was a commit in a fork.. Will find another correct commit..

JeroenKnoops commented 1 year ago

One cli test is still randomly failing due to timeouts.

  1) cli
       runs repolinter on a remote git repository:
     Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (D:\a\repolinter\repolinter\tests\cli\cli_tests.js)
      at listOnTimeout (node:internal/timers:568:17)
      at process.processTimers (node:internal/timers:511:7)

Will investigate this..

hyandell commented 1 year ago

Just noting that conceptually this seems good to me :)

Brend-Smits commented 1 year ago

We have tested this quite extensively in the past weeks on the Philips fork. Everything is working well. There is still some flakyness, especially with the Windows based tests, but it's way better than it was previously. This gets a thumbs up from me. I would appreciate an extra review from either @hyandell or @zhaoyuheng200

hyandell commented 1 year ago

I'm good to go with this; any improvement is great. Merging as GitHub UI claims it's fine (despite my making some package.json changes).