Skip to content

Use single quotes to make regression tests work on Windows #5580

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 13, 2020
Merged

Use single quotes to make regression tests work on Windows #5580

merged 1 commit into from
Nov 13, 2020

Conversation

tautschnig
Copy link
Collaborator

test.pl cannot cope with double quotes on some Windows-based platforms
(including GitHub's Windows environment).

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@codecov
Copy link

codecov bot commented Nov 13, 2020 •
edited
Loading

Codecov Report

Merging #5580 (92e3249) into develop (53750fa) will decrease coverage by 37.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #5580       +/-   ##
============================================
- Coverage    69.32%   32.27%   -37.06%     
============================================
  Files         1241      983      -258     
  Lines       100406    83377    -17029     
============================================
- Hits         69604    26907    -42697     
- Misses       30802    56470    +25668
Flag Coverage Δ
cproversmt2 ?
regression ?
unit 32.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/cpp/cpp_id.h 0.00% <0.00%> (-100.00%) ⬇️
src/cpp/cpp_scope.h 0.00% <0.00%> (-100.00%) ⬇️
src/cpp/cpp_token.h 0.00% <0.00%> (-100.00%) ⬇️
src/cpp/cpp_name.cpp 0.00% <0.00%> (-100.00%) ⬇️
src/cpp/cpp_util.cpp 0.00% <0.00%> (-100.00%) ⬇️
src/util/identifier.h 0.00% <0.00%> (-100.00%) ⬇️
src/xmllang/graphml.h 0.00% <0.00%> (-100.00%) ⬇️
src/cpp/cpp_is_pod.cpp 0.00% <0.00%> (-100.00%) ⬇️
src/cpp/template_map.h 0.00% <0.00%> (-100.00%) ⬇️
src/ansi-c/designator.h 0.00% <0.00%> (-100.00%) ⬇️
... and 944 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53750fa...92e3249. Read the comment docs.

martin-cs
martin-cs approved these changes Nov 13, 2020
View reviewed changes
Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

Changes as described. I will take your word that they are necessary.

NlightNFotis
NlightNFotis approved these changes Nov 13, 2020
View reviewed changes
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

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

Hi Michael,

Again many thanks for reactivating the tests on windows.

Good to go, just one problem with the jbmc-json-ui/pointer-simplification test passing the wrong syntax to jq on UNIX systems, causing jq to throw syntax errors.

jbmc/regression/jbmc-json-ui/pointer-simplification/test.desc Outdated
@@ -1,6 +1,6 @@
CORE
test
'.[] | select(has("result")) | .result | .[] | select(has("trace")) | .trace | .[] | select(has("value")) | .value | select(.name == "pointer") | {has_data: has("data")}' --json-ui --trace
'.[] | select(has('result')) | .result | .[] | select(has('trace')) | .trace | .[] | select(has('value')) | .value | select(.name == 'pointer') | {has_data: has('data')}' --json-ui --trace
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be tripping up all the UNIX builds now (ubuntu and osx).

Is there a way perhaps to have two of these commands, and dispatch based on OS detection at test runtime? I'm not sure we have one, but maybe it wouldn't be hard to come up with one?

If that's not possible/convenient for the time being, maybe this change could be reverted and the test marked as KNOWNBUG on windows?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for digging into this one, I was quite confused.

@tautschnig
Use single quotes to make regression tests work on Windows
92e3249 
test.pl cannot cope with double quotes on some Windows-based platforms
(including GitHub's Windows environment).
@tautschnig tautschnig merged commit a503f5c into diffblue:develop Nov 13, 2020
@tautschnig tautschnig deleted the single-quotes branch November 13, 2020 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NlightNFotis NlightNFotis NlightNFotis approved these changes

@martin-cs martin-cs martin-cs approved these changes

Assignees

@smowton smowton

@NlightNFotis NlightNFotis

Labels
bugfix Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tautschnig @NlightNFotis @martin-cs @smowton