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

Fix measure unit tests #137

Merged
merged 9 commits into from
Mar 12, 2024
Merged

Fix measure unit tests #137

merged 9 commits into from
Mar 12, 2024

Conversation

mdahlhausen
Copy link
Contributor

Pull request overview

A combination of several unit test fixes to pass the CI.

Pull Request Author

This pull request makes changes to (select all the apply):

  • Documentation
  • Infrastructure (includes singularity image, buildstock batch, dependencies, continuous integration tests)
  • Sampling
  • Workflow Measures
  • Upgrade Measures
  • Reporting Measures
  • Postprocessing

Author pull request checklist:

  • Tagged the pull request with the appropriate label (documentation, infrastructure, sampling, workflow measure, upgrade measure, reporting measure, postprocessing) to help categorize changes in the release notes.
  • Added tests for new measures
  • Updated measure .xml(s)
  • Register values added to comstock_column_definitions.csv
  • Both options_lookup.tsv files updated
  • 10k+ test run
  • Change documentation written
  • Measure documentation written
  • ComStock documentation updated
  • Changes reflected in example .yml files
  • Changes reflected in README.md files
  • Added 'See ComStock License' language to first two lines of each code file
  • Implements corresponding measure tests and indexing path in test/measure_tests.txt or/and test/resource_measure_tests.txt
  • All new and existing tests pass the CI

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a code review on GitHub
  • All related changes have been implemented: data and method additions, changes, tests
  • If fixing a defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • Reviewed change documentation
  • Ensured code files contain License reference
  • Results differences are reasonable
  • Make sure the newly added measures has been added with tests and indexed properly
  • CI status: all tests pass

ComStock Licensing Language - Add to Beginning of Each Code File

# ComStock™, Copyright (c) 2023 Alliance for Sustainable Energy, LLC. All rights reserved.
# See top level LICENSE.txt file for license terms.

ChristopherCaradonna and others added 5 commits March 6, 2024 11:53
- rename out of date method
- remove temporary code to fix indoor pipe ambient temps (fixed by NREL/EnergyPlus#9650)
@mdahlhausen mdahlhausen added workflow measure PR improves or adds baseline workflow measures upgrade measure PR improves or adds upgrade measures Pull Request - Ready for CI labels Mar 8, 2024
@mdahlhausen mdahlhausen requested a review from eringold March 8, 2024 17:29
Copy link
Contributor

@eringold eringold left a comment

Choose a reason for hiding this comment

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

Remove la_100_qaqc from https://github.com/NREL/ComStock/blob/main/ymls/bsb-integration-test-baseline.yml to prevent buildstock_local test failure

@mdahlhausen mdahlhausen requested a review from eringold March 11, 2024 16:33
@mdahlhausen
Copy link
Contributor Author

@eringold removed that line from the .yml

@eringold
Copy link
Contributor

eringold commented Mar 11, 2024

@mpraprost I see one error still on create_typical_building_from_model_test.rb:

  1) Error:
CreateTypicalBuildingFromModel_Test#test_generic_gbxml:
NoMethodError: undefined method `each' for false:FalseClass
    :/ruby/2.7.0/gems/openstudio-standards-0.5.0/lib/openstudio-standards/standards/Standards.Model.rb:2218:in `model_differentiate_primary_secondary_thermal_zones'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/resources/os_lib_model_generation.rb:3058:in `block (2 levels) in typical_building_from_model'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/resources/os_lib_model_generation.rb:3055:in `each'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/resources/os_lib_model_generation.rb:3055:in `block in typical_building_from_model'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/resources/os_lib_model_generation.rb:3028:in `each'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/resources/os_lib_model_generation.rb:3028:in `typical_building_from_model'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/measure.rb:493:in `run'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/tests/create_typical_building_from_model_test.rb:73:in `apply_measure_to_model'
    C:/Repos/NREL/ComStock/resources/measures/create_typical_building_from_model/tests/create_typical_building_from_model_test.rb:196:in `test_generic_gbxml'

-1 🍬

@eringold eringold merged commit 7159705 into main Mar 12, 2024
@eringold eringold deleted the fix/unit_tests branch March 12, 2024 15:21
@mdahlhausen mdahlhausen changed the title Fix unit tests Fix measure unit tests Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pull Request - Ready for CI upgrade measure PR improves or adds upgrade measures workflow measure PR improves or adds baseline workflow measures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants