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

Manager cannot handle corrupted scenario files. #240

Closed
ArekKuczynski opened this issue May 16, 2024 · 7 comments
Closed

Manager cannot handle corrupted scenario files. #240

ArekKuczynski opened this issue May 16, 2024 · 7 comments
Labels
bug Something isn't working Manager Issue Affects the Manager
Milestone

Comments

@ArekKuczynski
Copy link
Collaborator

Describe the bug
When you open a corrupted scenario file in Manager it is unable to repair the scenario file and get information's form it.

To Reproduce
Steps to reproduce the behavior:

  1. Create corrupted scenario.
  2. Try to load it in Manager.
  3. See error.

Expected behavior

  • Script should convert index number to correct one and collect every scenario data. For this example, 7 to 1.

Screenshots

  • When you try to open it:
    image

  • After restarting program, the Manager will reopen with the scenario file:
    image

Additional context
test.rfs file:

[Scenario]
uploadmode = err
scriptcount = 3
graphlist = 

[7]
robots = 71
delay = 72
rampup = 73
run = 74
test = Example Test Case
script = example.robot
excludelibraries = row7el
robotoptions = row7ro
testrepeater = True
injectsleepenabled = True
injectsleepminimum = 70
injectsleepmaximum = 75
disableloglog = True
disablelogreport = True
disablelogoutput = True
@ArekKuczynski ArekKuczynski added potential bug Might be a bug, requires further clarification Manager Issue Affects the Manager labels May 16, 2024
ArekKuczynski added a commit to NiceProjectPoland/rfswarm that referenced this issue May 16, 2024
@damies13
Copy link
Owner

I slightly disagree with the expected behavior, what you've shown here is what I would expect as the behavior for this scenario file, as long as the GUI doesn't crash and remains usable this working as it should.
By remains usable, I mean (in either first load or second load examples):

  • user is able to make changes
  • user is able to add and remove rows
  • user is able to operate all buttons, fields and menus
  • user is able to save and close
  • User can close without saving and scenario is unchanged

Because the the data for "Example Test Case" is row 7 but the scenario says only 3 rows should be there, row 7 should not be loaded, If a user manually modifies a scenario to remove rows, they also should update the row number of the row data to reflect the row position they want it to have
If scenario says only 7 or more rows, and the first 6 are missing, I'd expect the 7th one to be loaded first, 8th one to be loaded second, etc, and they should only get renumbers if the user saves the scenario.

  • Did you actually experience a crash? by crash I mean the UI was locked up.
  • Did you get stack trace errors on the console?
  • Perhaps this could be documented better? but this is not a normal usage scenario, the GUI should never produce a scenario file like this.

@ArekKuczynski
Copy link
Collaborator Author

Hmm i think you're right, the Manager should work like this and if the user decides to modify the scenario file, he must be careful with the structure. I also found one more thing: If i load this scenario file, the Manager will add extra row.

[Scenario]
uploadmode = err
scriptcount = 8
graphlist = 

[4]
robots = 22
delay = 23
rampup = 24
run = 25
test = Example Test Case
script = example.robot

[8]
robots = 71
delay = 72
rampup = 73
run = 74
test = Example Test Case
script = example.robot
excludelibraries = row7el
robotoptions = row7ro
testrepeater = True
injectsleepenabled = True
injectsleepminimum = 70
injectsleepmaximum = 75
disableloglog = True
disablelogreport = True
disablelogoutput = True

image

@ArekKuczynski
Copy link
Collaborator Author

ArekKuczynski commented May 16, 2024

Ah it should also work like this the first index is not zero :)

@damies13
Copy link
Owner

No it's not zero, because if the first row in the GUI had an index of 0 it would look weird, and I wanted the index in the file to match what it shows in the GUI.
I actually thought long and hard about that back in the early versions, probably spent most of a day if not more thinking about what it should be.

@ArekKuczynski
Copy link
Collaborator Author

ArekKuczynski commented May 17, 2024

  • Okay, I was testing corrupted scenarios again but this time with correct scriptcount, and when I created a file like this:
[Scenario]
uploadmode = err
scriptcount = 7
graphlist = 

[4]
robots = 22
delay = 23
rampup = 24
run = 25
test = Example Test Case4
script = example4.robot
excludelibraries = row4el
robotoptions = row4ro
testrepeater = True
injectsleepenabled = True
injectsleepminimum = 70
injectsleepmaximum = 75
disableloglog = True
disablelogreport = True
disablelogoutput = True

[7]
robots = 71
delay = 72
rampup = 73
run = 74
test = Example Test Case7
script = example7.robot
excludelibraries = row7el
robotoptions = row7ro
testrepeater = True
injectsleepenabled = True
injectsleepminimum = 30
injectsleepmaximum = 60
disableloglog = True
disablelogreport = True
disablelogoutput = True

This is a result, also the settings form first row disappeared and Manager did not crash:
image
image

Looks like the core.OpenFile() function is generating an error message when trying to get settings data from row.
And i think the Manager should work well with transferring indexes to right order, but if you add settings that error occurs.

  • When you create scenario file without settings like this:
[Scenario]
uploadmode = err
scriptcount = 7
graphlist = 

[4]
robots = 22
delay = 23
rampup = 24
run = 25
test = Example Test Case4
script = example4.robot

[7]
robots = 71
delay = 72
rampup = 73
run = 74
test = Example Test Case7
script = example7.robot

There will be not any errors in terminal but Manager will add extra row.
image

  • One thing i wonder about is if anyone will ever create this type of scenario. When someone modifies it, they should be aware of any small mistakes in scenario structure.

@damies13
Copy link
Owner

image

This is something the manager should handle more gracefully, it should check first if the row index data exists, it looks like it is for the options in the run screen but not for the options in the settings screen.

Good find 👍

@damies13 damies13 added bug Something isn't working and removed potential bug Might be a bug, requires further clarification labels May 18, 2024
@damies13 damies13 added this to the v1.3.1 milestone May 18, 2024
damies13 added a commit that referenced this issue May 19, 2024
Test cases for Issue #58 and Issue #240 - Verify scenario file is updated correctly when scripts are removed and verify the manager handles corrupted scenario files.
ArekKuczynski added a commit to NiceProjectPoland/rfswarm that referenced this issue Jun 7, 2024
damies13 added a commit that referenced this issue Jun 8, 2024
Bug fix for Issue #240 - Manager cannot handle corrupted scenario files.
@damies13
Copy link
Owner

damies13 commented Jun 8, 2024

Merged into release branch v1.3.1 and all test passed

@damies13 damies13 closed this as completed Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Manager Issue Affects the Manager
Projects
None yet
Development

No branches or pull requests

2 participants