Skip to content

Commit

Permalink
Merge pull request #752 from oceanprotocol/issue-676-logs-test
Browse files Browse the repository at this point in the history
try debug and fix randomly logs failing test
  • Loading branch information
paulo-ocean authored Nov 20, 2024
2 parents 1e43f70 + 6c4f53f commit 1dfedc2
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/components/database/BaseDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export abstract class AbstractLogDatabase {
moduleName?: string,
level?: string,
page?: number
): Promise<Record<string, any>[] | null>
): Promise<Record<string, any>[]>

abstract delete(logId: string): Promise<void>
abstract deleteOldLogs(): Promise<number>
Expand Down
17 changes: 6 additions & 11 deletions src/components/database/ElasticSearchDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ export class ElasticsearchLogDatabase extends AbstractLogDatabase {
moduleName?: string,
level?: string,
page?: number
): Promise<Record<string, any>[] | null> {
): Promise<Record<string, any>[]> {
try {
const filterConditions: any = {
bool: {
Expand Down Expand Up @@ -888,10 +888,6 @@ export class ElasticsearchLogDatabase extends AbstractLogDatabase {
from
})

console.log('logs results:', result)
console.log('logs results hits:', result.hits)
console.log('logs results hits hits:', result.hits.hits)

return result.hits.hits.map((hit: any) => {
return normalizeDocumentId(hit._source, hit._id)
})
Expand All @@ -903,7 +899,7 @@ export class ElasticsearchLogDatabase extends AbstractLogDatabase {
GENERIC_EMOJIS.EMOJI_CROSS_MARK,
LOG_LEVELS_STR.LEVEL_ERROR
)
return null
return []
}
}

Expand Down Expand Up @@ -943,13 +939,12 @@ export class ElasticsearchLogDatabase extends AbstractLogDatabase {
try {
const oldLogs = await this.retrieveMultipleLogs(new Date(0), deleteBeforeTime, 200)

if (oldLogs) {
for (const log of oldLogs) {
if (log.id) {
await this.delete(log.id)
}
for (const log of oldLogs) {
if (log.id) {
await this.delete(log.id)
}
}

return oldLogs ? oldLogs.length : 0
} catch (error) {
DATABASE_LOGGER.logMessageWithEmoji(
Expand Down
6 changes: 3 additions & 3 deletions src/components/database/TypenseDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ export class TypesenseLogDatabase extends AbstractLogDatabase {
moduleName?: string,
level?: string,
page?: number
): Promise<Record<string, any>[] | null> {
): Promise<Record<string, any>[]> {
try {
let filterConditions = `timestamp:>=${startTime.getTime()} && timestamp:<${endTime.getTime()}`
if (moduleName) {
Expand Down Expand Up @@ -851,7 +851,7 @@ export class TypesenseLogDatabase extends AbstractLogDatabase {
GENERIC_EMOJIS.EMOJI_CROSS_MARK,
LOG_LEVELS_STR.LEVEL_ERROR
)
return null
return []
}
}

Expand Down Expand Up @@ -902,7 +902,7 @@ export class TypesenseLogDatabase extends AbstractLogDatabase {
}
}
}
return oldLogs ? oldLogs.length : 0
return oldLogs.length
} catch (error) {
DATABASE_LOGGER.logMessageWithEmoji(
`Error when deleting old log entries: ${error.message}`,
Expand Down
2 changes: 1 addition & 1 deletion src/components/httpRoutes/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ logRoutes.post('/logs', express.json(), validateRequest, async (req, res) => {
.getDatabase()
.logs.retrieveMultipleLogs(startTime, endTime, maxLogs, moduleName, level, page)

if (logs) {
if (logs.length > 0) {
res.json(logs)
} else {
res.status(404).send('No logs found')
Expand Down
38 changes: 19 additions & 19 deletions src/test/integration/logs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ describe('LogDatabase CRUD', () => {
if (logs.length > 0) {
logs = logs.filter((log) => log.message === newLogEntry.message)

expect(logs?.length).to.equal(1)
expect(Number(logs?.[0].id)).to.greaterThan(Number(logId))
expect(logs?.[0].level).to.equal(newLogEntry.level)
expect(logs?.[0].message).to.equal(newLogEntry.message)
expect(logs?.[0].moduleName).to.equal('HTTP')
expect(logs.length).to.equal(1)
expect(Number(logs[0].id)).to.greaterThan(Number(logId))
expect(logs[0].level).to.equal(newLogEntry.level)
expect(logs[0].message).to.equal(newLogEntry.message)
expect(logs[0].moduleName).to.equal('HTTP')
}
})

Expand All @@ -118,11 +118,11 @@ describe('LogDatabase CRUD', () => {
logs = logs.filter((log) => log.message === newLogEntry.message)

if (logs.length > 0) {
expect(logs?.length).to.equal(1)
expect(Number(logs?.[0].id)).to.greaterThan(Number(logId))
expect(logs?.[0].level).to.equal(newLogEntry.level)
expect(logs?.[0].message).to.equal(newLogEntry.message)
expect(logs?.[0].moduleName).to.equal('HTTP')
expect(logs.length).to.equal(1)
expect(Number(logs[0].id)).to.greaterThan(Number(logId))
expect(logs[0].level).to.equal(newLogEntry.level)
expect(logs[0].message).to.equal(newLogEntry.message)
expect(logs[0].moduleName).to.equal('HTTP')
}
})

Expand Down Expand Up @@ -153,11 +153,11 @@ describe('LogDatabase CRUD', () => {
logs = logs.filter((log) => log.message.includes(newLogEntry.message))

if (logs.length > 0) {
expect(logs?.length).to.equal(1)
expect(Number(logs?.[0].id)).to.greaterThan(Number(logId))
expect(logs?.[0].level).to.equal(newLogEntry.level)
assert(logs?.[0].message)
expect(logs?.[0].moduleName).to.equal('HTTP')
expect(logs.length).to.equal(1)
expect(Number(logs[0].id)).to.greaterThan(Number(logId))
expect(logs[0].level).to.equal(newLogEntry.level)
assert(logs[0].message)
expect(logs[0].moduleName).to.equal('HTTP')
}
})

Expand Down Expand Up @@ -293,13 +293,13 @@ describe('LogDatabase retrieveMultipleLogs with specific parameters', () => {

it('should return an empty array for negative maxLogs', async () => {
const logs = await database.logs.retrieveMultipleLogs(startTime, endTime, -1)
assert.isNull(logs, 'Expected logs to be null')
assert.isEmpty(logs, 'Expected logs to be empty')
})

it('should retrieve a maximum of one log when maxLogs is set to 1', async () => {
const logs = await database.logs.retrieveMultipleLogs(startTime, endTime, 1)
// check if the length of logs is 1 or less
expect(logs?.length).to.be.at.most(1)
expect(logs.length).to.be.at.most(1)
})

it('should retrieve no logs when maxLogs is set to 0', async () => {
Expand Down Expand Up @@ -383,14 +383,14 @@ describe('LogDatabase deleteOldLogs', () => {
let logs = await database.logs.retrieveMultipleLogs(startTime, endTime, 100)

// Check that the old log is not present, but the recent one is
const oldLogPresent = logs?.some((log) => log.message === oldLogEntry.message)
const oldLogPresent = logs.some((log) => log.message === oldLogEntry.message)
assert(oldLogPresent === false, 'Old logs are still present')

// since we have many logs going to DB by default, we need to re-frame the timestamp to grab it
startTime = new Date(recentLogEntry.timestamp - 1000)
endTime = new Date(recentLogEntry.timestamp + 1000)
logs = await database.logs.retrieveMultipleLogs(startTime, endTime, 100)
const recentLogPresent = logs?.some((log) => log.message === recentLogEntry.message)
const recentLogPresent = logs.some((log) => log.message === recentLogEntry.message)
assert(recentLogPresent === true, 'Recent logs are not present')
} else
assert(
Expand Down

0 comments on commit 1dfedc2

Please sign in to comment.