Skip to content

Conversation

@GNL10
Copy link
Contributor

@GNL10 GNL10 commented Oct 29, 2025

No description provided.

@firewave
Copy link
Collaborator

Please add a test.

@firewave
Copy link
Collaborator

firewave commented Nov 6, 2025

Thanks but a unit test does not seem to make sense here as it is not clear how actual inputs can trigger this behavior.

Also the unit test just calls the function triggering the error should might not represent as it actually occurs in the executors. So it might be tested cases which will never occur.

@GNL10
Copy link
Contributor Author

GNL10 commented Nov 6, 2025

Thanks but a unit test does not seem to make sense here as it is not clear how actual inputs can trigger this behavior.

Also the unit test just calls the function triggering the error should might not represent as it actually occurs in the executors. So it might be tested cases which will never occur.

You are right, but looking at it, the only place where it would make sense to test that empty file, would be the getErrorMessages UT.
But this UT results in 333 messages in errorLogger.errmsgs (at the current commit). The second position in the iterator of errorLogger.errmsgs would be the one where we could apply the validation. But it looks strange validating only one position, and the second one at that.

I added an update, so you can see how it would look like. But I, myself, am not too happy about how it looks like.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2025

else
msg << " configurations.\n";

msg << "The checking of the file will be interrupted because there are too many "
Copy link
Owner

Choose a reason for hiding this comment

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

This was not caused by you. But the verbose message does not say how many configurations are checked or what the maxConfigs options is and that is unfortunate.

@danmar
Copy link
Owner

danmar commented Nov 7, 2025

Thanks but a unit test does not seem to make sense here as it is not clear how actual inputs can trigger this behavior.

imho the idea of unit tests is to test each function individually. And I like to unit test functions.

Also the unit test just calls the function triggering the error should might not represent as it actually occurs in the executors. So it might be tested cases which will never occur.

well it would be interesting to know if this can be reproduced. Spontanously I don't understand why this function would ever be called when numberOfConfigurations is <= maxConfigs .. can you @GNL10 share some info about this?

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