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

Add tests for Goal #217

Merged
merged 22 commits into from
Apr 6, 2021
Merged

Add tests for Goal #217

merged 22 commits into from
Apr 6, 2021

Conversation

lue97
Copy link
Collaborator

@lue97 lue97 commented Apr 5, 2021

No description provided.

@lue97 lue97 added the type.Chore Something that needs to be done, but not a story, bug, or an epic. e.g. Move testing code into a new label Apr 5, 2021
@codecov-io
Copy link

codecov-io commented Apr 6, 2021

Codecov Report

Merging #217 (a7e718f) into master (4c1ecaa) will increase coverage by 0.23%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #217      +/-   ##
============================================
+ Coverage     60.82%   61.06%   +0.23%     
- Complexity      716      722       +6     
============================================
  Files           135      135              
  Lines          2943     2943              
  Branches        324      324              
============================================
+ Hits           1790     1797       +7     
+ Misses         1025     1019       -6     
+ Partials        128      127       -1     
Impacted Files Coverage Δ Complexity Δ
src/main/java/seedu/address/model/person/Goal.java 85.18% <0.00%> (+12.96%) 24.00% <0.00%> (+6.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c1ecaa...a7e718f. Read the comment docs.

@lue97 lue97 changed the title Add tests for goals and methods in Person concerning goals Add tests for Goal Apr 6, 2021
@ivantjh
Copy link
Collaborator

ivantjh commented Apr 6, 2021

How did u manage to solve the error ah?

Copy link
Collaborator

@ivantjh ivantjh left a comment

Choose a reason for hiding this comment

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

lgtm!


public class GoalTest {

private static final List<String> VALID_WEEK_INPUTS = Arrays.stream(new String[]{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Arrays.asList(new String[] {"w", "week", "weekly"}) shorter line to convert to List


@Test
public void goal_emptyGoal_success() {
assertEquals(new Goal(Goal.Frequency.NONE), new Goal());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use static import for the whole file, will be more readable


@Test
public void goal_getFrequency_success() {
assertEquals(Goal.Frequency.NONE, new Goal(Goal.Frequency.NONE).getFrequency());
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should test something other than NONE since the default value is NONE


import org.junit.jupiter.api.Test;

public class EmailTest {
Copy link
Collaborator

@ivantjh ivantjh Apr 6, 2021

Choose a reason for hiding this comment

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

Why are we removing this? This seems valid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad this was unintended thanks for catching it.

@lue97
Copy link
Collaborator Author

lue97 commented Apr 6, 2021

How did u manage to solve the error ah?

I was working on a out of date branch and was making use of EventBuilder which was removed in another PR.

@ivantjh
Copy link
Collaborator

ivantjh commented Apr 6, 2021

Ohh ok

@lue97 lue97 merged commit 958e5d5 into AY2021S2-CS2103T-W14-1:master Apr 6, 2021
@lue97 lue97 mentioned this pull request Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type.Chore Something that needs to be done, but not a story, bug, or an epic. e.g. Move testing code into a new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants