-
Notifications
You must be signed in to change notification settings - Fork 236
test(monorepo): Make sure all tests are green COMPASS-4704 #2174
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
Conversation
…ack config; Ignore lib
… webpack config; Ignore lib
…bpack config; Ignore lib
…ck config; Ignore lib
… traces being present
…move sync fs methods
…odel as it is handled by driver (again)
…window context when possible instead of global
…y check the label as the test case description suggests
…x invalid project value test by providing value that is 100% invalid (for now)
…sts so that mocha tests can run; Add all missing dev deps
…; Skip broken test that uses mock-fs for now
"mongodb-ace-autocompleter": "^0.4.12", | ||
"mongodb-ace-mode": "^0.4.0", | ||
"mongodb-ace-theme-query": "0.0.2", | ||
"mongodb-client-encryption": "^1.2.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit surprising – the driver should not require this to be present unless we’re actually doing anything with FLE … are we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah, no, I don't think so... Let me double-check that I actually got the reason for this right in the first place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, so seems it's a breaking* change introduced in driver here https://github.com/mongodb/node-mongodb-native/pull/2760/files#diff-c088451b2b6c514ba0f163c7c00dfae50dc86c534feeffe18cf618749af66da0R15
* - breaking only for webpack because it tries to bundle mongodb-client-encryption and because this time it's a top level import it throws instead of warning as it does with v3.6.3
… so that we can npm ci everything
Yikes, so I guess we can't really run tests with GitHub Actions for all the packages until we get to normalizing and hoisting packages dependencies du -hs compass-monorepo
27G compass-monorepo
|
This PR ensures that every test suite in every package is green. It doesn't mean that all of them are brought up to date or improved, we are just trying to create a good baseline for all the future work. Every package gets its own commit with a fix, I'll additionally add some context for the changes that I had to make in every package
directConnection
and broke all our tests that assert that driver doesn't add it. Rolled back all our changes related todirectConnection
global
instead ofglobal.window
when running tests, those are somehow explicitly not the same when running mocha tests even with jsdom registered and this causedjsdom
to throw a bunch of obscure errors. Fixed by prioritizingwindow
overglobal
when creating dnd "backend"mongodb-client-encryption
npm run start
works (kinda sad thatcompass-home
is not a window into how compass looked ~ a year ago or even more)mock-fs
dependency, it's not a quick fix so the test suite is just disabled for now