Bi 46 review 1 #25
Merged
Bi 46 review 1 #25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## BI-46 #25 +/- ##
===========================================
+ Coverage 93.10% 100.00% +6.89%
===========================================
Files 4 3 -1
Lines 29 26 -3
Branches 3 4 +1
===========================================
- Hits 27 26 -1
+ Misses 2 0 -2 ☔ View full report in Codecov by Sentry. |
bacpactech
reviewed
Feb 7, 2024
| @@ -14,48 +16,48 @@ const query = gql` | |||
| } | |||
| } | |||
| ` | |||
|
|
|||
| let uri = 'https://web-app-client-b69l4yjrq-bacpacs-projects.vercel.app/api/graphql' | |||
There was a problem hiding this comment.
Add a comment such that:
// TODO: Figure out a way to _not_ use a deployed vercel instance for GraphQL
src/__tests__/query.test.js
Outdated
| const client = new ApolloClient({ | ||
| link: new HttpLink({ uri, fetch }), | ||
| cache: new InMemoryCache(), | ||
| }) | ||
| async function fetchData() { | ||
| try { | ||
| const result = await client.query({ query }) |
There was a problem hiding this comment.
Instead of using a try-catch, we can use this statement directly inside the tests. If we do that, then the tests will fail, if the fetching fails.
src/__tests__/query.test.js
Outdated
| } catch (e) { | ||
| expect(e.message).toMatch('error') | ||
| } | ||
| const result = await fetchData() |
src/__tests__/query.test.js
Outdated
| } catch (e) { | ||
| expect(e.message).toMatch('error') | ||
| } | ||
| const result = await fetchData() |
| describe('searchAlgorithm', () => { | ||
| it('returns filtered results when input and data are provided', () => { | ||
| const input = 'mass' | ||
| const colleges = [{ name: 'Massachusetts Institute of Technology (MIT)', country: 'United States', city: 'Cambridge' }] |
There was a problem hiding this comment.
Let's reuse the colleges in all these three tests. So, let's initialize it inside the describe.
| }) | ||
| it('returns an empty array when either input is not provided', () => { | ||
| const input = '' | ||
| const resultWithoutData = searchAlgorithm(input, null) |
There was a problem hiding this comment.
Let's separate the setup lines, act and assertions by adding blank lines below and above the act.
There was a problem hiding this comment.
You might want to rename the variable resultWithoutData to result, once you decide to use the colleges variable as data.
src/__tests__/query.test.js
Outdated
| @@ -14,48 +16,48 @@ const query = gql` | |||
| } | |||
| } | |||
| ` | |||
|
|
|||
| let uri = 'https://web-app-client-b69l4yjrq-bacpacs-projects.vercel.app/api/graphql' | |||
| console.log('graphql_uri', uri) | |||
There was a problem hiding this comment.
We can use the console log as:
console.log("GraphQL URI in tests:", uri)
src/client.js
Outdated
| // default: | ||
| // break | ||
| // } | ||
| console.log('graphql_uri', uri) |
There was a problem hiding this comment.
We can use the console log as:
console.log("GraphQL URI in Client:", uri)Remove the commented out lines.
src/queries/queries.js
Outdated
| // query getUniversityName($id: ID!) { | ||
| // university_name(id: $id) | ||
| // } | ||
| // ` |
There was a problem hiding this comment.
You can remove these commented out lines.
bacpactech
pushed a commit
that referenced
this pull request
Feb 8, 2024
* search-bar feature added * suggested changes added * 2nd review * merge commit fix * prettier formate * changes in stories * export removed * suggested changes made * storybook for navbar and footer is added * footer position changed from sticky to relative * css changes in footer * change in css of footer * change-1:-spacing,footer title resolved and font style corrected * design changed in navbar acc. to design team * navbarlinks colour changed to black * error resolved * homepage storybook added * changes based on chromatic comments * size of navbar and footer decresed * alignment correction * another text alignment correction * navbar alignment changes * navbar logo size fixed * searchbar element visibility corrected && other elemnts of searchpage added * design and responsiveness added * color added in specific texts * after review suggested changes made * fetching data from mongodb query added * change for BI-28 * query for fetching data from mongo is added, testing is left * error in workflow checks and tests resolved * storybook error solved * error in workflow resolved * workflow error is resolved * testing code for fetching college data is added * fixes to solve workflow error * testing code fixed * some error fixed * Bi 46 review 1 (#25) * changes made after review-1 * test error fixed * fixing testing error * fixing error in testing * fixing error of testing * fixing errors in testing query url * adding manual url * prev code to fix the error * deployed link added * Deployed link is added * vercal bot link added for test * after 2nd code review * switch statement is added for diffrent enviorments * absolute url in testing added * solving bi-46 issues * testing for apis * new link added * cors added * resolving error * test error resolved * prettier format checked * changes after 2nd review done
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
new pr for BI-46