SeleniumLibrary icon indicating copy to clipboard operation
SeleniumLibrary copied to clipboard

Imporve the indexing function for the pages screenshot.

Open AmirHdm opened this issue 1 year ago • 7 comments

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

AmirHdm avatar Jun 15 '23 23:06 AmirHdm

@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 avatar Jun 23 '23 12:06 emanlove

@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

AmirHdm avatar Jun 23 '23 17:06 AmirHdm

@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.

lisacrispin avatar Jun 23 '23 20:06 lisacrispin

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

AmirHdm avatar Jun 23 '23 20:06 AmirHdm

@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 avatar Jun 23 '23 23:06 emanlove

@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 avatar Jun 24 '23 00:06 AmirHdm

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

  • filename: str = 'selenium-screenshot-{index}.png'
  • filename: str = 'selenium-screenshot-{random}.png'
  • filename: str = 'selenium-screenshot-{timestamp}.png'

AmirHdm avatar Jul 04 '23 16:07 AmirHdm