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

Tests setUp and tearDown: void #2267

Merged
merged 5 commits into from
Sep 26, 2019
Merged

Conversation

MGatner
Copy link
Member

@MGatner MGatner commented Sep 26, 2019

Description
setUp() and tearDown() already always return void, but as of PHPUnit 8 the interface requires this in the declaration. This updates the test cases and all tests to add the explicit reference.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@jim-parry
Copy link
Contributor

jim-parry commented Sep 26, 2019

hmm - a couple of files changed here :-/

@MGatner
Copy link
Member Author

MGatner commented Sep 26, 2019

Good old recursive sed doing work! We'll see how the tests look, but I think it's an innocuous change.

@MGatner
Copy link
Member Author

MGatner commented Sep 26, 2019

I'm baffled what happened to the test view files. I didn't touch them - maybe some style enforcement injected the newline? It's causing the tests to fail. I'm trying a manual update via GitHub UI but failing that the test might need to be updated.

@jim-parry
Copy link
Contributor

I find that the commit pre-hook can mangle the formatting. Maybe it needs to be made smarter or more context-sensitive.

@MGatner
Copy link
Member Author

MGatner commented Sep 26, 2019

Yeah it's definitely breaking this - I think I will update the test for now because there is no way to commit those files without the newline.

@jim-parry
Copy link
Contributor

I have seen this before ... running a group of tests will have \n added to output, whereas running the entire suite doesn't, or vice-versa (can't remember which way it goes).
Anytime I run into this, I make sure the tests work when the lot of them is run, and avoid running that one module's.
This could be a phpunit thing!?

@MGatner
Copy link
Member Author

MGatner commented Sep 26, 2019

Not sure but I might redo this and exclude those files from the original commit to prevent them changing.

@MGatner
Copy link
Member Author

MGatner commented Sep 26, 2019

It looks like that fixed it. @jim-parry Do you feel okay about the tests embracing the newlines?

@jim-parry
Copy link
Contributor

I am ok with that - it is a different issue to track down at some point.

@MGatner MGatner merged commit 56717ad into codeigniter4:develop Sep 26, 2019
@MGatner MGatner deleted the tests-void branch October 22, 2019 16:04
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.

2 participants