search

aspnet / MusicStore

[Archived] MusicStore test application that uses ASP.NET/EF Core. Project moved to https://github.com/aspnet/AspNetCore
1.3k stars 878 forks source link

Smoke test behaviour change #649

Closed JunTaoLuo closed 8 years ago

JunTaoLuo commented 8 years ago

@kichalla I believe your changes in https://github.com/aspnet/MusicStore/pull/647 broke the smoke tests. I have made a fix in https://github.com/aspnet/MusicStore/commit/30ab5d2d8f6dff71045244b0213e1760ac954ff6 but I want to check with you so I'm opening this issue.

Your change on line https://github.com/aspnet/MusicStore/pull/647/files#diff-caf0051a47e53865ffb9052c1ab86d3bL49 removed the call to log the sample user in. This will cause a failure a few lines later on https://github.com/aspnet/MusicStore/pull/647/files#diff-caf0051a47e53865ffb9052c1ab86d3bL55 where the test tries to logout before the user logged in and fails during validation. It isn't clear why we need to test logout without ever logging in and I see that login and logout functionalities are tested later on during the test so I removed it in my fix.

Adding back the original login call can also work, but I don't see what purpose that would serve. I don't think testing duplicate user registration requires you to be logged in.

Let me know if there needs to be a follow up to the fixes I made. Otherwise if the fix looks good to you, feel free to close this issue.

JunTaoLuo commented 8 years ago

@kichalla and I talked about this offline, seems like it was the right fix, closing for now.