Skip to content

Fix: multistring assertions #266

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

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Fix: multistring assertions #266

merged 2 commits into from
Jun 18, 2024

Conversation

Chemaclass
Copy link
Member

@Chemaclass Chemaclass commented Jun 18, 2024

📚 Description

Related: https://github.com/phpstan/phpstan-src/actions/runs/9567238902/job/26374488151

Thanks for the support and help @staabm

🔖 Changes

  • Remove the optional label and use always label="$(helper::normalize_test_function_name "${FUNCNAME[1]}")", which is something we are using all around and it makes sense to keep it this way
  • For string arguments, use all arguments but the first one (reserved for the expected)

⏰ Disclaimer

  • assert_equals on multiline strings does not work, because multilines takes mutiple arguments positions when passing to other functions... so it's bash - at least I didn't find another solution for this problem so far.
  • This means that, multiline strings will work as last argument as they are treated as variadic arguments like ...string, therefore the first argument must be one liner, whatever that is.

You can use assert_contains or assert_matches instead

@staabm
Copy link
Contributor

staabm commented Jun 18, 2024

assert_equals on multiline strings does not work, because multilines takes mutiple arguments positions when passing to other functions... so it's bash - at least I didn't find another solution for this problem so far.

Would it make sense to emit an error in case of multi-line strings telling the user that its not supported?

@Chemaclass
Copy link
Member Author

@staabm, I wish that would be possible (or know how to do that...), however, at this point:

function assert_equals() {
  local expected="$1"
  local actual="$2"
  # ... etc ...

The issue is that there is no possibility to know if the second argumnet is a line that belong the text from the first argument or is a real new argument. Does it make sense? Therefore, for example, in here:

function assert_contains() {
  local expected="$1"
  local actual_arr=("${@:2}")
  local actual=$(printf '%s\n' "${actual_arr[@]}")

  if ! [[ $actual == *"$expected"* ]]; then
  # ... etc

what I am doing is to gather together all args (starting from the 2nd), store it into actual_arr and then concat all items with a new line \n into actual and so I can manipulate them easier. Does it makes sense and helps understand the intrinsic issue coming from bash? At least, I didn't find another way around to make this work so far... Maybe at a future time we find a better way to do this, but for now, I think it's good enough if this works in your PHPStan CI 😄

@Chemaclass Chemaclass merged commit 2599176 into main Jun 18, 2024
7 checks passed
@Chemaclass Chemaclass deleted the fix/multistring-assertions branch June 18, 2024 17:24
@staabm
Copy link
Contributor

staabm commented Jun 18, 2024

I see, thanks for the details. maybe it makes sense to open a new issue about this known problem, so people showing up in the issue tracker find the "known problem"..?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants