Skip to content

Variable in test.desc file can be also separated by ':' or ';' #3889

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

Merged
merged 1 commit into from
Jan 22, 2019
Merged

Variable in test.desc file can be also separated by ':' or ';' #3889

merged 1 commit into from
Jan 22, 2019

Conversation

forejtv
Copy link
Contributor

@forejtv forejtv commented Jan 22, 2019

To be able to use variables in .desc files also in the --classpath argument, we need to use : as a separator for variable names, currently the only separator is a space.

For example, we want to be able to use a .desc file in which the arguments have the following structure:

--classpath $PATH_TO_DEPENDENCIES:$PATH_TO_IMPORTANT_JAR:.

@forejtv forejtv requested review from thk123 and allredj January 22, 2019 10:02
@thk123
Copy link
Contributor

thk123 commented Jan 22, 2019

Sadly neither the commit message nor the PR description really enlighten me as to what is going on here? - perhaps a supporting example would help?

@forejtv
Copy link
Contributor Author

forejtv commented Jan 22, 2019

Sorry @thk123. I've added an example to the description.

@@ -74,6 +74,7 @@ ($$$$$$$$$$)
foreach my $key (@keys) {
my $value = $defines->{$key};
$options =~ s/(\$$key$|\$$key )/$value /g;
$options =~ s/\$$key:/$value:/g;
Copy link
Contributor

Choose a reason for hiding this comment

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

For maintainability, I would love a comment to explain why we need the : as a separator, e.g.

# Variables in --classpath are separated by colons

Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

🚫
This PR failed Diffblue compatibility checks (cbmc commit: 9d9023a).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98159435
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.

Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

I'm confused. Doesn't --classpath '../../../../scripts/format_classpath.sh $PATH1 $PATH2' do what you want? (' should be backtick)

@forejtv forejtv changed the title Variable in test.desc file can be also separated by ':' Variable in test.desc file can be also separated by ':' or ';' Jan 22, 2019
@forejtv forejtv merged commit be2c068 into diffblue:develop Jan 22, 2019
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

✔️
Passed Diffblue compatibility checks (cbmc commit: f127bf4).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98200668

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.

4 participants