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

Unit test scaffolding for CueJobMonitorTree and Redirect. #680

Merged

Conversation

bcipriano
Copy link
Collaborator

Link the Issue(s) this Pull Request is related to.
#396

Summarize your change.
No real test logic, but sets up the widgets and some basic data. The aim here is two-fold:

  1. Get a cheap boost to our code coverage. Even though we're not validating results this at least ensures the file syntax and basic setup is functional.
  2. Make it easier for folks to add tests in the future -- adding one or two tests is easy enough if the basic widget/mock structure is already in place, but starting that from scratch is a high enough barrier to entry to discourage test writing.

This brings CueGUI coverage 33% -> 38%.

I hope to bring this approach to other parts of the codebase in followup PRs.

Copy link
Collaborator

@gregdenton gregdenton left a comment

Choose a reason for hiding this comment

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

LGTM. One minor comment, your choice to implement.

cuegui.Style.init()

show_name = 'arbitrary-show-name'
job_name = 'arbitrary-job-name'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Slight preference: It looks like job_name is only used in the ShowGetJobWhiteboardResponse call which passes it as a list. Can we just define jobs here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@bcipriano bcipriano merged commit 03c08a8 into AcademySoftwareFoundation:master Apr 30, 2020
@bcipriano bcipriano deleted the test-scaffolding branch April 30, 2020 23:13
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
  翻译: