Skip to content

Conversation

@ekpyron
Copy link
Collaborator

@ekpyron ekpyron commented Mar 16, 2018

…x tests.

Fixes #3742.

Are there any other naming schemes for temporary files that will still result in temporary files being considered valid tests?

@ekpyron ekpyron self-assigned this Mar 16, 2018
@ekpyron ekpyron requested a review from chriseth March 16, 2018 10:46
@ekpyron ekpyron force-pushed the syntaxTestsExtensions branch from 8395da9 to df39015 Compare March 16, 2018 10:52
Copy link
Contributor

@chriseth chriseth left a comment

Choose a reason for hiding this comment

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

Please reduce code duplication.

fs::directory_iterator()
))
numTestsAdded += registerTests(*sub_suite, _basepath, _path / entry.path().filename());
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

Please format as

if (fs::is_directory(entry.path()) || (
    ... &&
))
``

if (
fs::is_directory(entry.path()) ||
(entry.path().extension().string() == ".sol" &&
!boost::starts_with(entry.path().filename().string(), "~"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I thought about excluding files that start with ..

@ekpyron ekpyron force-pushed the syntaxTestsExtensions branch from df39015 to e68c19c Compare March 16, 2018 11:28
@ekpyron
Copy link
Collaborator Author

ekpyron commented Mar 16, 2018

@chriseth Requested changes done: moved filename filter to a static function in SyntaxTest; also filter files starting with ".". Code formatting request becomes obsolete.

@chriseth chriseth merged commit 2b2527f into develop Mar 16, 2018
@axic axic deleted the syntaxTestsExtensions branch March 21, 2018 18:06
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.

3 participants