Attachment support #3
Conversation
lamkovod
force-pushed
the
attachment-support
branch
from
e89fbbf to
c720da6
Compare
index.js
Outdated
| @@ -339,6 +352,20 @@ function addTest(testTitle, testCaseId, buildUrl) { | |||
| } | |||
| } | |||
|
|
|||
| function getAttachmentPath(attachmentPath, testCaseId) { | |||
| const files = getFiles(attachmentPath) | |||
There was a problem hiding this comment.
it is possible that multiple tests have the same testCaseId.
Is it possible to get screenshot name directly from test object? It would reduce complexity a lot.
There was a problem hiding this comment.
Well, we could use full test name and default path without subfolders, but I'm pretty sure that it all fully depends on user's testing framework settings.
Ex. : Cypress got it's own subfolder for each spec file.
But still full test name can be used for screenshot search to make usage of "shared" testCaseId possible.
There was a problem hiding this comment.
Most frameworks can generate full path to screenshots, not sure if cypress can do it.
That would help a lot
index.js
Outdated
|
|
||
| const attachmentsFolder = path.join(process.cwd(), `${attachmentFolder}`) | ||
|
|
||
| if (fs.existsSync(attachmentsFolder)) { |
There was a problem hiding this comment.
I'm fine with the approach, however you may also take a look at https://github.com/adamgruber/mochawesome#addcontexttestobj-context
It looks a bit more flexible imo.
We can have global after each hook, in case test failed add attachment to test object and then extract it here.
That's up to you
There was a problem hiding this comment.
Can you please test and provide some examples with following frameworks:
- cypress
- pure mocha framework
The way it is implemented currently doesn't look really well for me, however if it works with cypress/mocha/wdio I don't mind.
I think that it would require storing screenshots in file system for mocha framework (without cypress). If yes - we'll need to think on different approach/solution.
Anyway, thank you for helping with this!
| @@ -339,6 +344,31 @@ function addTest(testTitle, testCaseId, buildUrl) { | |||
| } | |||
| } | |||
|
|
|||
| function getAttachment(attachmentFolder, testTitle) { | |||
| const attachmentsFolder = path.join(process.cwd(), `${attachmentFolder}`) | |||
There was a problem hiding this comment.
I think attachmentFolder can be declared once in the top.
Can't see any reason to pass attachmentFolder in current implementation
There was a problem hiding this comment.
Not sure, it's a property of qTestConfig
| const attachmentsFolder = path.join(process.cwd(), `${attachmentFolder}`) | ||
|
|
||
| if (fs.existsSync(attachmentsFolder)) { | ||
| const attachmentPath = getFiles(attachmentsFolder).find(filename => filename.includes(testTitle)) |
There was a problem hiding this comment.
can you please mention somewhere (in doc/readme) how are you attaching screenshots, please?
It is not obvious at all
index.js
Outdated
| if (attachmentPath) { | ||
| return fs.readFileSync(attachmentPath).toString('base64') | ||
| } else { | ||
| console.log("Attachments with qTestId in name not found") |
There was a problem hiding this comment.
Looks like the code is not properly formatted
index.js
Outdated
| const res = path.resolve(folderPath, dirent) | ||
| return fs.existsSync(res) && fs.lstatSync(res).isDirectory() ? getFiles(res) : res | ||
| }) | ||
| return Array.prototype.concat(...files) |
There was a problem hiding this comment.
are you converting files to array here? May it be simplified to return [...files]?
lamkovod
force-pushed
the
attachment-support
branch
from
32cbb61 to
6578a20
Compare
Proposed changes
Attachment support with possibility of choosing attachment media type.
Types of changes
New feature
Fixes #1