Skip to content

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 allredj and thk123 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.

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