Minor test updates for db-panel.test#2802
Conversation
448c699 to
8f4f391
Compare
| // This test has some serious problems: | ||
| // a) it runs twice: we couldn't find out why | ||
| // b) all tests use the same dbConfig file, hence the tests depend on ORDER and have to use the same list name! | ||
| // c) since we use a file watcher to update the config we sometimes need to wait (sleep) before accessing the config again | ||
| // d) we depend on highlighted list items when adding a repo to a list. If there's not enough time in between, a test might think a list is highlighted that doesn't exist anymore |
There was a problem hiding this comment.
I don't think these apply anymore. If there are still some that apply, I think it would be better to create a tech debt issue; I think documenting tech debt in comments can quickly lead to outdated comments or us losing track of this.
There was a problem hiding this comment.
Hm, this is still very much valid. It cost me a lot of time to figure all these tings out because they were not documented. Charis mentioned in the initial integration test PR that we should've had a comment there from the start #2690 (comment)
I think we're not intending to change this atm as well (so that a tech-dept issue wouldn't be a good place to store the information as well..).
There was a problem hiding this comment.
I agree that this is useful information to have. I think we can leave in points b and d, but I think we can remove points a and c:
- For point a, this is probably not specific to this test. If I run these tests using the CLI, I don't get duplicate test runs (
npm run test:vscode-integration:activated-extension -- --runTestsByPath test/vscode-tests/activated-extension/databases/db-panel.test.ts) - For point c, I don't see any sleep commands in the tests
There was a problem hiding this comment.
You're right, I just copy and pasted my initial comment without checking it again. Will make the changes you're suggesting!
8f4f391 to
9aff989
Compare
During investigations into a new code-search integration test some minor updates were found worth keeping.
Checklist
ready-for-doc-reviewlabel there.