test(NODE-6028): skip atlas connectivity free tier 4.4 #4041
Conversation
| if (process.env.ATLAS_CONNECTIVITY == null) { | ||
| console.error( | ||
| 'skipping atlas connectivity tests, ATLAS_CONNECTIVITY environment variable is not defined' | ||
| ); | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
Should I delete this? if we accidentally removed the env var then these tests would silently skip right?
There was a problem hiding this comment.
Agreed.
Did these tests live in tests/integration at any point? I'm wondering if is here to prevent failures on regular integration runs.
| let client; | ||
|
|
||
| afterEach(async function () { | ||
| await client.close(); |
There was a problem hiding this comment.
pulled the close up to an after each
| await client.close(); | ||
| }); | ||
|
|
||
| for (const configName of Object.keys(CONFIGS)) { |
| for (const configName of Object.keys(CONFIGS)) { | ||
| context(configName, function () { | ||
| for (const connectionString of CONFIGS[configName]) { | ||
| const name = connectionString.includes('mongodb+srv') ? 'mongodb+srv' : 'normal'; |
There was a problem hiding this comment.
indexOf -> includes
| const today = new Date(); | ||
| // Making this April 1st so it is a monday | ||
| const april1st2024 = new Date('2024-04-01'); | ||
| if (today < april1st2024) { |
There was a problem hiding this comment.
This is kinda fun TBH, if the upgrade doesn't happen we'll go red again otherwise this will just become dead code and we'll start testing it again 😎
nbbeeken
added
the
Primary Review
| const today = new Date(); | ||
| // Making this April 1st so it is a monday | ||
| const april1st2024 = new Date('2024-04-01'); | ||
| if (today < april1st2024) { |
| if (process.env.ATLAS_CONNECTIVITY == null) { | ||
| console.error( | ||
| 'skipping atlas connectivity tests, ATLAS_CONNECTIVITY environment variable is not defined' | ||
| ); | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
Agreed.
Did these tests live in tests/integration at any point? I'm wondering if is here to prevent failures on regular integration runs.
Description
What is changing?
Is there new documentation needed for these changes?
What is the motivation for this change?
Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript