Skip to content

Disable tests relying on ExpandObjects, when BUILD_FORTRAN is off#11036

Merged
Myoldmopar merged 1 commit intoNatLabRockies:developfrom
dominik-wojt-2311446:disable_tests_requiring_fortran
Jun 5, 2025
Merged

Disable tests relying on ExpandObjects, when BUILD_FORTRAN is off#11036
Myoldmopar merged 1 commit intoNatLabRockies:developfrom
dominik-wojt-2311446:disable_tests_requiring_fortran

Conversation

@dominik-wojt-2311446
Copy link
Contributor

@dominik-wojt-2311446 dominik-wojt-2311446 commented Apr 16, 2025

This can be reverted once #6566 is solved.

Pull request overview

Prevents unexpected test failures.

Description of the purpose of this PR

Prevent failures, when BUILD_FORTRAN is OFF.

Pull Request Author

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish

Reviewer

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@dominik-wojt-2311446
Copy link
Contributor Author

dominik-wojt-2311446 commented Apr 16, 2025

I can't add labels on my own. 'Defect' seems to be most applicable of the available labels.

@dominik-wojt-2311446 dominik-wojt-2311446 force-pushed the disable_tests_requiring_fortran branch from e277dc8 to 1967c82 Compare April 29, 2025 14:01
@dominik-wojt-2311446
Copy link
Contributor Author

dominik-wojt-2311446 commented Apr 29, 2025

I now put the tests together with other test having same issue.

@dominik-wojt-2311446
Copy link
Contributor Author

dominik-wojt-2311446 commented Apr 30, 2025

Hi @Myoldmopar , sorry for calling you out directly, but this PR probably got missed due to missing labels and review request - I have no privileges to add either.
This small fix prevents newcomers (like me) from seeing test failures after compiling the project with default cmake settings.

@Myoldmopar
Copy link
Member

Myoldmopar commented May 1, 2025

@dominik-wojt-2311446 I don't mind you calling me out directly, and I didn't miss this, it's just not made it into my mental (or physical) review queue yet. The reviews for things like this often come in spurts throughout the development cycle. This looks like it will be a quick review, I will get to it soon. Thank you!

@nrel-bot-2
Copy link

nrel-bot-2 commented May 30, 2025

@dominik-wojt-2311446 @Myoldmopar it has been 28 days since this pull request was last updated.

Myoldmopar
Myoldmopar approved these changes Jun 5, 2025
View reviewed changes
Copy link
Member

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

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

This is great @dominik-wojt-2311446

@Myoldmopar
Copy link
Member

Myoldmopar commented Jun 5, 2025

Thanks @dominik-wojt-2311446, contribution agreement received, and this is an obvious cleanup. Merging it now.

@Myoldmopar Myoldmopar merged commit a40f708 into NatLabRockies:develop Jun 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Myoldmopar Myoldmopar Myoldmopar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dominik-wojt-2311446 @Myoldmopar @nrel-bot-2