search

cartalyst / sentinel

A framework agnostic authentication & authorization system.
BSD 3-Clause "New" or "Revised" License
1.51k stars 238 forks source link

Better tests #474

Closed 8633brown closed 5 years ago

8633brown commented 5 years ago

these are some improved tests. all risky tests should now be fixed. I may still try to tidy them up a little more so i'm just putting a drft PR in for now. this is more for note on #473 for the tests i used to find it.

8633brown commented 5 years ago

Also just a side note. i noticed some of the other Cartalyst repo master branches have been getting pushed to the latest version of laravel and php. i was wondering if that is in the pipeline for this project too?

brunogaspar commented 5 years ago

Hey @8633brown

Yes, Sentinel will get those updates too :)

brunogaspar commented 5 years ago

Just checked the changes you did here, i would ask you to hold on doing more changes (so you don't waste more time on it) as i'm currently reorganising the tests and also fixing the ones that requires attention.

8633brown commented 5 years ago

Youre more than welcome to close this if you think its not relevant any more.

brunogaspar commented 5 years ago

I've pushed all the changes i wanted to do, at least for now heh.

Feel free to continue to do your awesome work of course, i totally appreciate your effort!

8633brown commented 5 years ago

Thanks. I'm relatively quite new to the programming stuff so it's at least a little confidence inspiring to view your changes and see i wasnt going too far wrong. i guess the only big difference i was going for was removing the mock connection from the Illuminate tests. and mocking the Eloquent class instead. as the connection side is being tested in the Eloquent tests anyway is this worth doing?

brunogaspar commented 5 years ago

is this worth doing?

Yes, absolutely!