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

Clean up test descriptions, addresses #372. #416

Merged
merged 5 commits into from
Jan 21, 2020
Merged

Conversation

ruebot
Copy link
Member

@ruebot ruebot commented Jan 21, 2020

GitHub issue(s): #372

What does this Pull Request do?

  • Clean up test descriptions
  • Rename typo filename
  • Remove potentially unnecessary tests

How should this be tested?

  • Copy editing
  • TravisCI

- Rename typo filename
- Remove potentially unnecessary tests
Copy link
Member

@ianmilligan1 ianmilligan1 left a comment

Choose a reason for hiding this comment

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

These look great (and I'll leave the the actual tests to TravisCI). Some half hearted comments re: copy editing, which you can take or leave. 😄

@codecov
Copy link

codecov bot commented Jan 21, 2020

Codecov Report

Merging #416 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #416   +/-   ##
=======================================
  Coverage   77.96%   77.96%           
=======================================
  Files          41       41           
  Lines        1570     1570           
  Branches      292      292           
=======================================
  Hits         1224     1224           
  Misses        218      218           
  Partials      128      128

@ruebot
Copy link
Member Author

ruebot commented Jan 21, 2020

Ok, I'll put those back in. I stand corrected.

Copy link
Member

@SamFritz SamFritz left a comment

Choose a reason for hiding this comment

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

Looks good here @ruebot. The notes in this review are more just questions to prompt a change if necessary.

@@ -43,21 +43,21 @@ class RecordRDDTest extends FunSuite with BeforeAndAfter {
sc = new SparkContext(conf)
}

test("no valid pages") {
test("Expect no valid pages") {
Copy link
Member

Choose a reason for hiding this comment

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

should this also include RDD at the end?

src/test/scala/io/archivesunleashed/RecordRDDTest.scala Outdated Show resolved Hide resolved
src/test/scala/io/archivesunleashed/RecordRDDTest.scala Outdated Show resolved Hide resolved
@ianmilligan1 ianmilligan1 merged commit ffef735 into master Jan 21, 2020
@ianmilligan1 ianmilligan1 deleted the issue-372-tests branch January 21, 2020 22:14
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.

3 participants