Skip to content

Mocking lab#12

Open
obna wants to merge 8 commits into
MillsMCS:mocking-basefrom
obna:mocking-lab
Open

Mocking lab#12
obna wants to merge 8 commits into
MillsMCS:mocking-basefrom
obna:mocking-lab

Conversation

@obna
Copy link
Copy Markdown

@obna obna commented Dec 19, 2020

No description provided.

@ellenspertus ellenspertus changed the base branch from complete to mocking-base December 22, 2020 04:08
@ellenspertus
Copy link
Copy Markdown
Contributor

Omoremi, I'm not sure if you followed my instructions to view your PR while/after creating it in GitHub. There are 48 changed files. There should be only a few.

You can fix this by git rm the files that don't belong (generated javadoc) and adding doc/ to .gitignore.

As always, let me know if you don't understand my instructions.

Copy link
Copy Markdown
Contributor

@ellenspertus ellenspertus left a comment

Choose a reason for hiding this comment

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

You can improve this through Tuesday, Dec. 22.

Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/WordRecord.java Outdated
ObservableList<WordRecord> testListRecord = FXCollections.observableList(testList);
SampleData.addWordOfTheDay(testListRecord, mockWordsApi, mockWordApi);

WordOfTheDay wordToday = SampleData.getWordOfTheDay(mockWordsApi);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't call getWordOfTheDay() in this test. You are testing addWordOfTheDay(). Only use that. Base your assertions on the WordRecord that is added to the list.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

not sure what this means

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should not call getWordOfTheDay() in this method, which exists to test addWordOfTheDay(). The latter method adds a WordRecord to testListRecord. You should be calling getWord() and other methods on that WordRecord.

Please let me know if you understand now.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes I think so

Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
assertEquals("jingle", wordToday.getWord());
assertEquals(DEF_LIST, wordToday.getDefinitions());
assertEquals(187, SampleData.getFrequencyByYear(mockWordApi, A_WORD, 2020));
assertEquals("jingle", TODAYS_WORD.getWord());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you misunderstood my comment. Your code was right before this commit when it called SampleData.getWordOfTheDay(). As it is now written, this does not test getWordOfTheDay() because it does not call it.

Copy link
Copy Markdown
Contributor

@ellenspertus ellenspertus left a comment

Choose a reason for hiding this comment

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

Good job addressing the javadoc issues.

/**
* Adds word records to a passed list.
*
* @param backingList the list of word records
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wouldn't include type information (that the list contains word records) since that is shown automatically in generated javadoc as part of the signature.

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