Skip to content

Github test - #8

Open
tatiana-017 wants to merge 1 commit into
mainfrom
githubTest
Open

Github test#8
tatiana-017 wants to merge 1 commit into
mainfrom
githubTest

Conversation

@tatiana-017

@tatiana-017 tatiana-017 commented Jul 27, 2022

Copy link
Copy Markdown
Owner

image

Comment thread package.json
Comment on lines +38 to +40
"chai-json-schema": "^1.5.1",
"isomorphic-fetch": "^3.0.0",
"md5": "^2.3.0"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be dev dependencies, run the following command to fix it:
npm i -D chai-json-schema isomorphic-fetch md5

chai.use(chaiSubset);

describe('Github Api Repositories Test', () => {
let responseData;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can rename it as userDataResponse

Comment thread test/GithubApi.Repositories.test.js
expect(response.data.name).to.equal(data.name);
expect(response.data.location).to.equal(data.location);
expect(response.data.company).to.equal(data.company);
responseData = response;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is redundant you don't need the response const


describe('Github Api Repositories Test', () => {
let responseData;
let repoName;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of repoName you can rename it as jasmineRepo or something similar

});

describe('Consume POST and PATCH service on Github', () => {
let loggedIn;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is more descriptive userData

}
};

let gists;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to rename it as createdGistResponse or similar

it('Gists verification', async () => {
gistsInf = await auth.get(gists.data.url);

expect(gistsInf.status).to.be.equal(StatusCodes.OK);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one expect to verify gist data returned and stored in gistInf is the same than created in previous test

Comment on lines +45 to +49
try {
await auth.get(gists.data.url);
} catch (err) {
expect(err.response.status).to.be.equal(StatusCodes.NOT_FOUND);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this way, the test only will pass if the error happens, but, if no error is retreived the test will pass but it should fail, you can create a variable, and then initialize it in the catch, and outside of the try-catch sentence perform the validation of the NOT_FOUND status

Comment on lines +86 to +90
try {
await fetch(gist.url, getBody);
} catch (err) {
expect(err.response.status).to.be.equal(StatusCodes.NOT_FOUND);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this way, the test only will pass if the error happens, but, if no error is retreived the test will pass but it should fail, you can create a variable, and then initialize it in the catch, and outside of the try-catch sentence perform the validation of the NOT_FOUND status

@holgiosalos

holgiosalos commented Aug 3, 2022

Copy link
Copy Markdown
Collaborator

OMG, you owe me a coffee for this large PR 😓

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