Skip to content

Ability to change path by overriding task #179

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mikstime
Copy link

No description provided.

src/commands.ts Outdated
@@ -162,6 +162,7 @@ Cypress.Commands.add(
},
log: false,
})
.then(() => cy.task(TASK.processImgPath, { path: imgPath }).then(newImgPath => imgPath = newImgPath))
.then(() => imgPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line won't be needed anymore

@FRSgit
Copy link
Member

FRSgit commented Nov 12, 2022

Hey @mikstime, thank you for this PR!
Code looks good, but I need help understanding why is it needed - can you elaborate a bit more on what and why is happening here 😅 ? (I've already read through your comment)

@mikstime
Copy link
Author

mikstime commented Nov 14, 2022

Hey @mikstime, thank you for this PR!
Code looks good, but I need help understanding why is it needed - can you elaborate a bit more on what and why is happening here 😅 ? (I've already read through your comment)

I didn't want to change plugin's behaviour but wanted to integrate this plugin with mochawesome reporter. By overriding after:screenshot hook and changing imgPath it is possible to do so.
Posibile implementation presented bellow.

const pathMapping = {};
const on2 = (action, handler) => {
    if (action === 'after:screenshot') {
        const newHandler = async props => {
            const originalPath = props.path;

            const newProps = await handler(props);
            if (newProps) {
                await fs.copyFile(newProps.path, originalPath);
                pathMapping[originalPath] = newProps.path;
            }
        };
        on(action, newHandler);
    } else if (action === 'task') {
        handler[TASK.processImgPath] = ({ path }) => pathMapping[path];
        on(action, handler);
    } else {
        on(action, handler);
    }
};

initPlugin(on2, config);

It is useful to know that by changing code above it is possible to replace image in the report file e.g combine screenshot and diff file together. I might or might not publish this as a separate package in the near future

@mikstime mikstime requested a review from FRSgit November 14, 2022 12:35
@FRSgit
Copy link
Member

FRSgit commented Jun 5, 2023

Hey!
I think I came to a.moment when I'm ready to tackle this one, sorry that it took so long.

I'm just wondering - currently there is a possibility to leave the images in the original screenshots directory by using imagesPath configuration option. Can you confirm if that could already help with a mock awesome situation?

But either way - this change make sense, I'll approve it and merge soon. I'm also thinking about providing a mochawesome recipe:
If I understand correctly your code is just copying over the image files back to the original directory, right?

@mikstime
Copy link
Author

mikstime commented Jun 7, 2023

If I understand correctly your code is just copying over the image files back to the original directory, right?

It's been a while since I finished with this problem. I believe that is exactly what it does.

I've tried implementing a proper solution for this problem and ended up rewriting plenty of code since there was a lot of unclear logic related to absolute/relative paths. Currently i can't share my solution.

I'm just wondering - currently there is a possibility to leave the images in the original screenshots directory by using imagesPath configuration option. Can you confirm if that could already help with a mock awesome situation?

Sorry, short on time currently. Won't try this one on my own.

@FRSgit FRSgit force-pushed the main branch 9 times, most recently from 3ea7824 to b215cc3 Compare June 17, 2023 21:01
@FRSgit FRSgit mentioned this pull request Sep 14, 2024
22 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants