isoltest does not support filenames (dots)

[axic]

This seems to be not too intuitive:

$ test/tools/isoltest -t smtCheckerTests/control_flow/revert.sol
Invalid test unit filter - can only contain '[a-zA-Z1-9_/*]*: smtCheckerTests/control_flow/revert.sol

While of course -t smtCheckerTests/control_flow/* or -t smtCheckerTests/control_flow/revert* works.

[aaroosh-07]

Hi, can I take up this issue. and can you guide me on how to debug this issue

[cameel]

@aaroosh-07 Sure! Do you have the environment set up and can you run isoltest already? If not, please take a look at these:

• https://docs.soliditylang.org/en/latest/installing-solidity.html#building-from-source
• https://docs.soliditylang.org/en/latest/contributing.html#running-the-compiler-tests

Once you have it all set up and want some pointers specific for the task please come over to the #solidity-dev channel and we can discuss it there.

[chriseth]

What is the actual goal here? -t is used to specify test unit names. It would be nice if also files could be tested directly, but do we really want to use -t for that? Also in that case, should the string be interpreted as a path relative to the current working directory?

[cameel]

I thought it was to make isoltest work with both -t smtCheckerTests/control_flow/revert and -t smtCheckerTests/control_flow/revert.sol. Currently only the first form works. I can see how this might not make sense in the general case but for our .sol and .yul test cases in standalone files the name of the test always matches the path (just with the extension stripped).

I think that the simplest solution would be to include the extension in test case names when we register them. This would not affect normal boost test cases cases. For the ones in standalone files -t smtCheckerTests/control_flow/revert would no longer work but both -t smtCheckerTests/control_flow/revert* and -t smtCheckerTests/control_flow/revert.sol would.

[cameel]

By the way, @aaroosh-07 looks like I was too quick with assigning this issue. It's not in the implementation backlog yet which means that we did not really agree on whether we actually want it and in what form. We need to discuss it first.

[aaroosh-07]

okay @cameel. I think we can make documentation more beginner friendly by adding hard coded relative paths for libevmone.so in Running complier tests\Prerequisite

[cameel]

@aaroosh-07 @christianparpart has already opened a PR for that yesterday: #11613. If you have any feedback, please join the review :)

Anyway, this issue is blocked for the time being because we need a decision so what do you think about taking a different one? Maybe one of these?

• #10299
• #11583
• #11415

[chriseth]

I think we should support calling isoltest directly on single files - regardless of where isoltest is called from, i.e. isoltest bla.sol (without -t).

Maybe we can split this into

a) renaming all our file-based tests to include the .sol ending (this should be easy). b) allowing isoltest to take direct arguments and somehow figure out what kind of test it is (by looking in the containing directory, for example) and then run the test.

[cameel]

a) sounds good.

With b), which one do you mean (I'd assume option 1 but figure out what kind of test it is suggests something more complex, like in option 2):

1. Accepting a path to the test relative to the current dir so that instead of

   isoltest --test=syntaxTests/array/concat/bytes_concat_*.sol
   

   you could do just

   isoltest bytes_concat_*.sol
   

   as long as you're already inside syntaxTests/array/concat/?

   This sounds like something nice to have, though I'm not sure how useful it would be in practice (for example I rarely cd into test dirs, not sure about other people). It would make it a clear distinction between files and test names though.

2. Or maybe that

    isoltest bytes_concat_*.sol
   

   would just figure out where tests matching bytes_concat_*.sol are located and act as if you specified the whole path, regardless of where you are in the filesystem?

   This would be definitely more useful though you can already do something close:

   isoltest --test=*/bytes_concat_*
   

   With soltest too, but there you have to know exactly how deeply the file is nested:

   soltest --run_test=*/*/*/bytes_concat_*
   

   You cannot specify the file extension here but doing a) would make it possible.

[cameel]

We talked about this very briefly on today's call. Basically, this is just a small usability improvement so @chriseth is not too particular about the details.

I'd say, let's do a) and as for b) let's go with option 1, which is in line with a similar change I recently did in cmdlineTests.sh (https://github.com/ethereum/solidity/pull/11712).

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 90 days. It will be automatically closed in 7 days.

[github-actions[bot]]

Hi everyone! This issue has been automatically closed due to inactivity. If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen. However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.