robotframework / SeleniumLibrary

Web testing library for Robot Framework
Apache License 2.0
1.36k stars 751 forks source link

Imporve the indexing function for the pages screenshot. #1840

Open AmirHdm opened 1 year ago

AmirHdm commented 1 year ago

Add indexing functionality based on datetime for capture screen page method inside ScreenshotKeywords class.

emanlove commented 1 year ago

@AmirHdm Just wanted to let you know that your pull request has been noticed and within the queue to be reviewed. My plan was to bring this to the team to review and discuss. I will update you sometime within the next couple weeks.

AmirHdm commented 1 year ago

@AmirHdm Just wanted to let you know that your pull request has been noticed and within the queue to be reviewed. My plan was to bring this to the team to review and discuss. I will update you sometime within the next couple weeks.

@emanlove great ! And thanks for the feedback

lisacrispin commented 1 year ago

@AmirHdm Is this changing the naming of screenshot files from simply numbering them sequentially based on the index, to using the timestamp in the name? That does seem like a good idea, if so.

AmirHdm commented 1 year ago

@lisacrispin yeath that's right because using timestamps in naming is more helpful and significantly

emanlove commented 1 year ago

@AmirHdm I like the idea of providing different type of "indexing" or filenames. One concern I do have though is breaking any existing systems that rely on numerical indexing. Relooking at the code and keyword documentation I see for singular Capture Page Screenshot there is a filename argument which allows one to format the filename. I'm curious as to whether we could use this "syntactical idea" to allow for something like filename = selenium-screenshot-{timestamp}.png or filename = selenium-screenshot-{timestamp:%Y-%m-%d-%H-%M-%S}.png or maybe even filename = selenium-screenshot-{random}.png and apply it to the default capture screenshot on error functionality?

AmirHdm commented 1 year ago

@AmirHdm I like the idea of providing different type of "indexing" or filenames. One concern I do have though is breaking any existing systems that rely on numerical indexing. Relooking at the code and keyword documentation I see for singular Capture Page Screenshot there is a filename argument which allows one to format the filename. I'm curious as to whether we could use this "syntactical idea" to allow for something like filename = selenium-screenshot-{timestamp}.png or filename = selenium-screenshot-{timestamp:%Y-%m-%d-%H-%M-%S}.png or maybe even filename = selenium-screenshot-{random}.png and apply it to the default capture screenshot on error functionality?

@emanlove sure i guess we could apply your idea and actually it's a great feature then let me see what i could do and will give a quick feedback

AmirHdm commented 1 year ago

The user now is able to set 3 indexing options on the keyword capture page screenshot :