-
-
Notifications
You must be signed in to change notification settings - Fork 362
v.colors: Added test cases #6105
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
base: main
Are you sure you want to change the base?
Conversation
Since the test file wasn’t collected and executed, I directly renamed the folder so we can see if they pass |
assert "255:0:0" in out | ||
assert "0:255:0" in out | ||
assert "0:0:255" in out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the order inside a rule file important? For example, if a parsed rule file has its output order shuffled, does it matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No the order is not important. The test case should work even when the order is shuffled. These lines are just checking the presence of Solid R, G and B values in the output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, according to this, the test using "in" for an assert makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order in output is important. It does matter which features has the color assigned. Just like in the other cases, use JSON to get a easy to process format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The json output for "v.db.select" is as follows:
------------------------------------------------ Captured stdout call ------------------------------------------------
{'info': {'columns': [{'name': 'GRASSRGB', 'sql_type': 'CHARACTER', 'is_number': False}]}, 'records': [{'GRASSRGB': '255:0:0'}, {'GRASSRGB': '0:255:0'}, {'GRASSRGB': '0:0:255'}]}
How do I consider the order in this case? Do you mean the value of R should be before G and G should be before B everytime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that the first color is for the first point, the second color for the second one, e.g. point 1 is light yellow, point 2 bright red...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I have fixed that in the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has a good spread of features it is testing (just seeing the test file, I didn't compare to the actual doc).
However, it requires more testing of the results. The existing and newly added JSON outputs will help with that and will also simplify the code.
In the meantime, #2923 is merged. It would be helpful to use it and test it that way. Using grass.tool will simplify the test code by replacing env=session.env
for each tool call by session=session
for each test function. It makes the code using file inputs simpler by providing a straightforward way for using stdin. Finally, it will make the JSON outputs easy to use.
out = gs.read_command( | ||
"v.db.select", map=mapname, columns="GRASSRGB", env=session.env | ||
) | ||
lines = out.strip().splitlines()[1:] | ||
assert lines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not test much and really under-uses the output from v.db.select. Use the JSON output format of v.db.select (format="json"
).
Given that #2923 is merged, you can use (more to try it out rather than grass.tools being necessary here):
tools = Tools(session=session)
data = tools.v_db_select(map=mapname, columns="GRASSRGB")
# now assert different things about *data*
In any case, you can test more that way without any custom parsing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have used grass.tools as suggested.
assert "255:0:0" in out | ||
assert "0:255:0" in out | ||
assert "0:0:255" in out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order in output is important. It does matter which features has the color assigned. Just like in the other cases, use JSON to get a easy to process format.
result = tools.v_colors_out(map=mapname, format="json") | ||
rules = json.loads(result.stdout) | ||
|
||
values = [str(rule["value"]) for rule in rules] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the conversion to str?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was for checking a numeric value later on. But I have changed the function to isinstance now.
No description provided.