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

Make tests null safe #904

Merged
merged 17 commits into from
Sep 10, 2023
Merged

Make tests null safe #904

merged 17 commits into from
Sep 10, 2023

Conversation

LuisDuarte1
Copy link
Member

Closes #513

This also updates flutter and dart sdk (because newer versions of mockito need it).

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@LuisDuarte1 LuisDuarte1 marked this pull request as draft August 10, 2023 21:04
@LuisDuarte1 LuisDuarte1 marked this pull request as ready for review August 21, 2023 15:38
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #904 (006e281) into develop (078d9e3) will increase coverage by 1%.
Report is 10 commits behind head on develop.
The diff coverage is 29%.

Additional details and impacted files
@@          Coverage Diff           @@
##           develop   #904   +/-   ##
======================================
+ Coverage       22%    22%   +1%     
======================================
  Files          136    136           
  Lines         3532   3536    +4     
======================================
+ Hits           746    760   +14     
+ Misses        2786   2776   -10     

Process-ing
Process-ing previously approved these changes Aug 22, 2023
Copy link
Contributor

@Process-ing Process-ing left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@bdmendes bdmendes left a comment

Choose a reason for hiding this comment

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

Haven't got a chance to review this thoroughly, but for now do not forget to update the flutter version in the deploy action too

@LuisDuarte1
Copy link
Member Author

Haven't got a chance to review this thoroughly, but for now do not forget to update the flutter version in the deploy action too

It was already done

@bdmendes
Copy link
Member

It's still 3.7.2 here, am I missing something?

@LuisDuarte1
Copy link
Member Author

It's still 3.7.2 here, am I missing something?

Sorry I forgot to push ahahah done

Process-ing
Process-ing previously approved these changes Aug 23, 2023
Copy link
Member

@bdmendes bdmendes left a comment

Choose a reason for hiding this comment

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

  1. Should we add documentation about the build runner step in uni/README.md?
  2. Should we exclude *.mocks.dart from linting and coverage analyzers?

@LuisDuarte1
Copy link
Member Author

LuisDuarte1 commented Aug 24, 2023

  1. Should we add documentation about the build runner step in uni/README.md?

I agree I'll do it, and maybe in the future we should write proper documentation on how to write tests on flutter, to help future contributors, but it should be done separately.

  1. Should we exclude *.mocks.dart from linting and coverage analyzers?

Not sure if coverage will be affected, since the coverage should only be analyzed in the lib/ folder. But I'll change the format/linting ones

@bdmendes
Copy link
Member

I'll make sure to write something about testing in my presentation slides later this September. We can then base the proper docs on it

Copy link
Member

@bdmendes bdmendes left a comment

Choose a reason for hiding this comment

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

Quick grammar police before getting this merged

uni/README.md Outdated Show resolved Hide resolved
uni/README.md Outdated Show resolved Hide resolved
uni/README.md Outdated Show resolved Hide resolved
bdmendes
bdmendes previously approved these changes Aug 25, 2023
Process-ing
Process-ing previously approved these changes Aug 31, 2023
Copy link
Member

@bdmendes bdmendes left a comment

Choose a reason for hiding this comment

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

Good to go 🚀

@LuisDuarte1 LuisDuarte1 merged commit 86a2078 into develop Sep 10, 2023
6 checks passed
@LuisDuarte1 LuisDuarte1 deleted the feature/test-null-safety branch September 10, 2023 10:01
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.

Migrate Mockito to easier null safety package
3 participants