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

Allow folders/directories in test cases #110

Closed
dmcdougall opened this issue Apr 17, 2023 · 8 comments
Closed

Allow folders/directories in test cases #110

dmcdougall opened this issue Apr 17, 2023 · 8 comments

Comments

@dmcdougall
Copy link
Contributor

dmcdougall commented Apr 17, 2023

This issue isn't really an issue in the true sense of the word. I'm just opening an issue because I wasn't sure where else to get some insight.

I noticed that passing test case locations that aren't in the current directory, and therefore contain a directory, are not supported. Is there a technical reason for this limitation? I use multidelta quite a bit. My workflows usually involve minimising complex applications which often come with source trees that aren't necessarily "flat", and multidelta is happy to accept testcases with folders in the path.

I'm happy to poke around in the cvise source code and contribute a patch that would allow users to pass test cases that contain directories, but I just wanted to make sure that there wasn't some historical context behind the limitation first. Perhaps someone has already tried this and it was too awkward to support? Any insight would be valuable.

@marxin
Copy link
Owner

marxin commented Apr 18, 2023

This issue isn't really an issue in the true sense of the word. I'm just opening an issue because I wasn't sure where else to get some insight.

First, thanks for the issue, appreciate that!

I noticed that passing test case locations that aren't in the current directory, and therefore contain a directory, are not supported. Is there a technical reason for this limitation?

Yes, the main limitation is that there are concurrent reduction candidates of the reduced files and each of them is copied into a temporary folder. And that's why C-Vise does not modify a source file(s) in place. However, I've faced similar challenge you have multiple times in the future and I always made a workaround:

  • -I location for include of header files
  • cp xxx . && gcc a.c ... - interestingness test can also copy files first
  • I hardcoded not modified locations with absolute paths: gcc a.c /tmp/[bcd].o

I use multidelta quite a bit. My workflows usually involve minimising complex applications which often come with source trees that aren't necessarily "flat", and multidelta is happy to accept testcases with folders in the path.

Fully makes sense.

I'm happy to poke around in the cvise source code and contribute a patch that would allow users to pass test cases that contain directories, but I just wanted to make sure that there wasn't some historical context behind the limitation first. Perhaps someone has already tried this and it was too awkward to support? Any insight would be valuable.

No, I haven't made any attempt and I would welcome a patch regarding this problem. I can imagine a new option that would make a copy of a given folder to the temporary file. Or do you have any better solution? Please, keep in mind the reduction happens in parallel and so an application folder can't interact in between parallel processes.

@dmcdougall
Copy link
Contributor Author

dmcdougall commented Apr 24, 2023

I've faced similar challenge you have multiple times in the future and I always made a workaround

Great minds think alike. I've also implemented similar workarounds for my use cases.

I can imagine a new option that would make a copy of a given folder to the temporary file. Or do you have any better solution?

That's the solution I had in mind. Take the user-provided test case, which contains a folder, and strip the filename. There are tools like basename that do this in UNIX so I'm sure there are spiritual equivalents provided by the path module in python. Then have cvise do the equivalent of mkdir -p <part of path containing only the directories> inside the temporary test directory and copy the test case there.

I can probably quite easily make a patch for this if you're interested.

@marxin
Copy link
Owner

marxin commented Apr 25, 2023

All right, so I made a prototype in the following branch:
https://github.com/marxin/cvise/tree/add--copy-extra-folder

It adds a new argument --copy-extra-folder that copies content (recursively) of a directory to a temporary place where inter. test is run. Note the reduced test-cases may be contained in the folder and it can be also . folder if you want.
Please test it and give me a feedback if it's feasible or not.

@dmcdougall
Copy link
Contributor Author

I didn't expect the entire contents of the tree to be copied over, just the test case and supporting folder structure. Maybe this doesn't matter, though.

@marxin
Copy link
Owner

marxin commented Apr 26, 2023

Oh, you expected only the supporting folder structure. So can you please reduce my patch candidate, probably rename the option, test it, and make a pull request that would fill your needs? Thanks.

@dmcdougall
Copy link
Contributor Author

Sure, I'd be happy to.

@dmcdougall
Copy link
Contributor Author

My attempt is in #132.

Constructive criticism is very welcome.

@dmcdougall
Copy link
Contributor Author

Fixed in #133

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

No branches or pull requests

2 participants