-
Notifications
You must be signed in to change notification settings - Fork 504
Add user logout routes to test files #1691
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
base: development
Are you sure you want to change the base?
Changes from 5 commits
4cc953f
13206a2
badd17e
5c3b0a5
7b6d28a
fa38778
9a67537
db34c64
714b4fb
6de5ae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -162,11 +162,20 @@ mocha.describe('meters API', () => { | |
| // Since this .before is in the middle of tests, it should not have issues as | ||
| // documented in usersTest.js. | ||
| mocha.before(async () => { | ||
| // login | ||
| let res = await chai.request(app).post('/api/loginLogout/login') | ||
| .send({ username: testUser.username, password: testUser.password }); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is supposed to test both ADMIN and CSV roles, then it looks like it doesn't access the CSV role. The test looks like it simply runs ADMIN both times instead of depending on the role being looped. If this behavior is unintended, then I can make the appropriate changes.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I see seems very similar to the next test for the other roles: mocha.describe('Admin role & CSV role:', () => {
for (const role in User.role) {
if (User.role[role] !== User.role.OBVIUS && User.role[role] !== User.role.EXPORT) {I see what you mean and you seem correct. I think it should login with the desired role and use that token. Thanks for offering to make that change. Unlike the next test which should test everyone else, this one should only test these roles. Thus, I think it would be much better to check if the role is admin or csv rather than not the others. Then it will still work if more roles are added. If you agree, could you also do that?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I adjusted the test to login with the same role, but I noticed that it would only pass if I changed the before hook to a beforeEach hook in I tested and found that the same error message is given for export and obvius roles for the same test, regardless of whether before or beforeEach is used (This might be the expected outcome, but shouldn't be for admin or csv). I also found that there's no difference between using an after or afterEach hook on line 176, but I kept it as afterEach to be consistent. I'm unsure of why I would strictly need a beforeEach for this test, but I plan on leaving it like this unless you have any input. |
||
| token = res.body.token; | ||
| }); | ||
| mocha.it('returns all meters', async () => { | ||
| mocha.after(async () => { | ||
| // logout | ||
| if (token) { | ||
| await chai.request(app).post('/api/loginLogout/logout') | ||
| .set('token', token); | ||
| } | ||
| }); | ||
|
|
||
| mocha.it(`should return all meters for ${role}`, async () => { | ||
| const conn = testDB.getConnection(); | ||
| await new Meter(undefined, 'Meter 1', '1.1.1.1', true, true, Meter.type.MAMAC, '+01', gps, | ||
| 'Identified 1', 'notes 1', 10.0, true, true, '01:01:25', '05:05:05', 5.1, 7.3, 1, 'increasing', false, | ||
|
|
@@ -217,7 +226,14 @@ mocha.describe('meters API', () => { | |
| .send({ username: unauthorizedUser.username, password: unauthorizedUser.password }); | ||
| token = res.body.token; | ||
| }); | ||
|
|
||
| mocha.afterEach(async () => { | ||
| // logout | ||
| if (token) { | ||
| await chai.request(app).post('/api/loginLogout/logout') | ||
| .set('token', token); | ||
| } | ||
| }); | ||
|
|
||
| mocha.it('should only return visible data', async () => { | ||
| const conn = testDB.getConnection(); | ||
| await new Meter(undefined, 'Meter 1', '1.1.1.1', true, true, Meter.type.MAMAC, '+01', gps, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.