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

core: add support for test case variants #227

Merged
merged 1 commit into from
Feb 16, 2018

Conversation

mwasilew
Copy link
Contributor

In some cases (Android CTS) test come with different variants. Test
variants are identified with [foo] at the end of the test name. The
string in the brackets can contain slashes.

Signed-off-by: Milosz Wasilewski milosz.wasilewski@linaro.org

In some cases (Android CTS) test come with different variants. Test
variants are identified with [foo] at the end of the test name. The
string in the brackets can contain slashes.

Signed-off-by: Milosz Wasilewski <milosz.wasilewski@linaro.org>
Copy link
Contributor

@danrue danrue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks OK and it should work well. I'm a little leery of adding what seems like CTS-specific requirements here, but I assume there is not a better place to do it.

name = '/'.join(parts[index:])
else:
group_name = '/'.join(parts[0:-1])
name = parts[-1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was skeptical that this algorithm worked in all cases but I added some tests and couldn't find any bugs (see danrue@b1df44c if you want to add them). Even so, I think it might have be simpler to pull out the [string] using a regex or similar rather than a loop.

@mwasilew
Copy link
Contributor Author

It comes from CTS (the problem we have), but doesn't have to be specific to CTS. I can imagine tests with variants (values vector) that are represented in brackets. The only problem is that we consider "/" as a special character. CTS doesn't do that and I didn't come up with any better idea how to solve the problem. We could propose the fix to CTS, but I doubt it would be accepted :)

@terceiro
Copy link
Member

the code looks ok to me as well. I am a bit concerned about having so support any arbitrary naming convention out in the world, but let's go for this one for now

@terceiro terceiro merged commit 808e6fa into Linaro:master Feb 16, 2018
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.

None yet

3 participants