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

lestarch: reverting enable UTs to OFF and a deprecation warning #682

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

LeStarch
Copy link
Collaborator

@LeStarch LeStarch commented Jun 7, 2021

Originating Project/Creator Infrastructure
Affected Component UTs
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n)
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

Make framework UTs enabled by default.

Rationale

Projects pointed out that framework UTs "are" UTs and thus should be run by default. Since this is not the best long-term way to handle this, we are also adding a deprecation warning.

Copy link
Contributor

@Joshua-Anderson Joshua-Anderson left a comment

Choose a reason for hiding this comment

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

My concern with this patch is that this makes disabling project unit tests, which aren't something most users want to run, an explicit step that users have to know about and remember to run every time while regenerating.

Nothing a user edits in their deployment should break framework unit tests. I kinda consider this akin to running all your python project dependencies unit tests in addition to your own - a little overkill for most projects, but a requirement to high reliability projects.

I totally understand that some projects may want to run these unit tests to verify the framework (probably in CI), but most of the projects that have those sort of requirements will likely know that they need to explicitly run the framework unit tests and setup their CI accordingly.

While I personally think this is likely not the best default behavior for F', I have no issue with being overruled here and the implementation looks good.

@LeStarch LeStarch merged commit 5ff62bd into nasa:devel Jun 8, 2021
@LeStarch LeStarch deleted the update/fix-default-enable-uts branch September 2, 2021 22:41
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