Skip to content
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

Cleaning in tests #727

Merged
merged 12 commits into from
Aug 2, 2022
Merged

Cleaning in tests #727

merged 12 commits into from
Aug 2, 2022

Conversation

@bidoubiwa bidoubiwa added skip-changelog The PR will not appear in the release changelogs refactor labels Apr 4, 2022
@bidoubiwa bidoubiwa changed the base branch from main to builder_like April 4, 2022 17:33
@bidoubiwa bidoubiwa marked this pull request as draft April 4, 2022 17:39
@bidoubiwa bidoubiwa marked this pull request as ready for review April 4, 2022 17:40
@bidoubiwa bidoubiwa requested a review from brunoocasali April 4, 2022 17:40
@bidoubiwa bidoubiwa marked this pull request as draft April 4, 2022 17:42
@bidoubiwa bidoubiwa marked this pull request as ready for review April 4, 2022 17:55
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Awesome! I found two things to be changed but everything else is ok!

Two really good references for you about testing good practices are:

bidoubiwa and others added 2 commits April 5, 2022 12:13
…ts.ts

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
…ts.ts

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
brunoocasali
brunoocasali previously approved these changes Apr 5, 2022
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Amazing work @bidoubiwa :)

Base automatically changed from builder_like to main July 26, 2022 08:27
@bidoubiwa bidoubiwa requested review from brunoocasali and mdubus July 26, 2022 14:52
Copy link
Member

@mdubus mdubus left a comment

Choose a reason for hiding this comment

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

LGTM 🥳

@bidoubiwa
Copy link
Contributor Author

bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Aug 2, 2022

@meili-bors meili-bors bot merged commit bf8ef3a into main Aug 2, 2022
@meili-bors meili-bors bot deleted the cleaning_in_tests branch August 2, 2022 09:38
defaultFacetDistribution: {},
finitePagination: false,
const DEFAULT_CONTEXT = {
indexUid: 'test',
Copy link
Member

Choose a reason for hiding this comment

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

I think the same idea can be applied here, don't you think? https://github.com/meilisearch/meilisearch-ruby/pull/354/files#r934317471

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think test is not a good index name? Also these are unit tests so they will not be done against meilisearch

expect(key).toEqual('""""""')
})
test('Test to format empty string', () => {
test('to format empty string', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think you missed this :)

meili-bors bot added a commit that referenced this pull request Aug 4, 2022
819: Remove redundant  words in tests description r=bidoubiwa a=bidoubiwa

as per this comment #727 (comment)

Co-authored-by: Charlotte Vermandel <charlottevermandel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor skip-changelog The PR will not appear in the release changelogs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants