Skip to content
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

Ability to change path by overriding task #179

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants