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 comstock-integrated test Jenkins settings #136

Merged
merged 8 commits into from
Mar 19, 2024
Merged

Fix comstock-integrated test Jenkins settings #136

merged 8 commits into from
Mar 19, 2024

Conversation

wenyikuang
Copy link
Collaborator

Pull request overview

Since the Comstock instance is initial s3 client in the constructor,
to run the Comstock integrated test, we must have a way to offer a
s3 client in order for computation.

And if we are using the "real" s3 client, the sso expiration will need us
update the authorization token every few hours.

I directly mock the s3 client (s3mixin) and read/write function in the
utitlity class in order to read from local disk.

The target of varification in the test is:

  • The buildstock_local could finish with yml/bsb-baseline
  • Postprocessing could initial the comstock and cebecs correctly with local files
  • The generated wide.csv is following the right shape with right count of run column.

Pull Request Author

Andrew, Wenyi

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 (CI run link: https://ci.openstudio.net/blue/organizations/jenkins/comstock-internal/detail/wenyi%2Fpr/12/pipeline)

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.

@wenyikuang wenyikuang added bug Something isn't working infrastructure PR improves or updates the build process and dependencies Pull Request - Ready for CI labels Mar 8, 2024
@wenyikuang wenyikuang closed this Mar 13, 2024
@wenyikuang wenyikuang reopened this Mar 13, 2024
@wenyikuang wenyikuang changed the title fix comstock-integrated test with mock s3 and local read function fix comstock-integrated test with proper setting in jenkins. Mar 13, 2024
@wenyikuang wenyikuang closed this Mar 13, 2024
@wenyikuang wenyikuang reopened this Mar 13, 2024
@wenyikuang
Copy link
Collaborator Author

wenyikuang commented Mar 14, 2024

After I rebase my commit onto the most recent main, it broken due to simulation_output_report.total_site_energy_mbtu missed. Need fix..

fixed, need catch up with the tag update in the ymls/ file.

@asparke2 asparke2 merged commit 00264f9 into main Mar 19, 2024
1 of 3 checks passed
@asparke2 asparke2 deleted the wenyi/pr branch March 19, 2024 12:34
@mdahlhausen mdahlhausen changed the title fix comstock-integrated test with proper setting in jenkins. Fix comstock-integrated test Jenkins settings Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working infrastructure PR improves or updates the build process and dependencies Pull Request - Ready for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix the s3 client issue in python tests
3 participants