-
Notifications
You must be signed in to change notification settings - Fork 15
add CoolProp development testing #52
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
Open
longemen3000
wants to merge
27
commits into
CoolProp:master
Choose a base branch
from
longemen3000:patch-3
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
ca35b83
add dev test
longemen3000 0a6d65e
typo
longemen3000 e762a5c
typo on yml
longemen3000 8ae40c2
try to run this test when the yml is modified
longemen3000 b4e7730
add arch
longemen3000 675b9c3
add arch (2)
longemen3000 7151061
fix yml
longemen3000 002c9d2
update CoolPropDevLoader uuid
longemen3000 07139a1
reduce to 20 min again
longemen3000 38dd7f2
big typo
longemen3000 966bb6d
typo
longemen3000 98b92ba
use changes suggested by llm
longemen3000 1536b15
typo
longemen3000 c6820ee
fix test loader
longemen3000 023c522
try with export_prefs = true
longemen3000 36f8d34
let's try this
longemen3000 596da78
try this (2)
longemen3000 a0079f3
try this (3)
longemen3000 7c08371
remove unused pkg develop
longemen3000 7a1ac1b
load same CoolProp_jll pkg
longemen3000 a35a7ce
try again
longemen3000 b588219
remove inaccurate version
longemen3000 022643b
change name in gh
longemen3000 825557c
.
longemen3000 2637c3d
remove unused constant
longemen3000 a7cded5
suggestions by coderabbit
longemen3000 5fb7614
restrict permissions
longemen3000 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| name: Development Tests | ||
|
|
||
| on: | ||
| schedule: | ||
| - cron: 0 0 * * * | ||
| workflow_dispatch: | ||
| push: | ||
| paths: | ||
| - '.github/**' | ||
| - 'test/CoolPropDevLoader/**' | ||
|
|
||
| concurrency: | ||
| # Skip intermediate builds: all builds except for builds on the `master` branch | ||
| # Cancel intermediate builds: only pull request builds | ||
| group: ${{ github.workflow }}-${{ github.ref }}-${{ github.ref != 'refs/heads/master' || github.run_number }} | ||
| cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }} | ||
|
|
||
| jobs: | ||
| test-coolprop-dev: | ||
| timeout-minutes: 30 | ||
| strategy: | ||
| matrix: | ||
| julia_version: | ||
| - '1.12' | ||
| os: | ||
| - ubuntu-latest | ||
| julia_arch: | ||
| - x64 | ||
| fail-fast: false | ||
| runs-on: ${{ matrix.os }} | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v7 | ||
|
|
||
| - uses: julia-actions/setup-julia@v3 | ||
| with: | ||
| arch: ${{ matrix.julia_arch }} | ||
| version: ${{ matrix.julia_version }} | ||
| - uses: julia-actions/cache@v3 | ||
|
|
||
| - name: add CoolPropDevLoader | ||
| shell: julia --color=yes --project=. {0} | ||
| run: | | ||
| using Pkg | ||
| Pkg.develop(path="test/CoolPropDevLoader") | ||
|
|
||
| - name: Compiling CoolProp | ||
| shell: julia --color=yes --compiled-modules=no --project=test {0} | ||
| run: | | ||
| using Pkg | ||
| Pkg.develop(path="test/CoolPropDevLoader") | ||
| using CoolPropDevLoader | ||
| CoolPropDevLoader.use_dev_library(CoolPropDevLoader.CompileCoolPropFromSourceforge()) | ||
| Pkg.add("Test") | ||
|
|
||
| - uses: julia-actions/julia-runtest@v1 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,4 +38,4 @@ using Unitful: °C, Pa | |
|
|
||
| HAPropsSI("H", "Tdb", 20°C, "RH", 0.5, "P", 101325Pa) | ||
| 38622.83892391293 J kg⁻¹ | ||
| ``` | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| name = "CoolPropDevLoader" | ||
| uuid = "1abf7821-c65a-4ba0-83a4-5660520f37c7" | ||
| version = "1.0.0" | ||
|
|
||
| [deps] | ||
| Downloads = "f43a241f-c20a-4ad4-852c-f6b1247861c6" | ||
| Preferences = "21216c6a-2e73-6563-6e65-726566657250" | ||
| CoolProp_jll = "3351c21f-4feb-5f29-afb9-f4fcb0e27549" | ||
|
|
||
| [compat] | ||
| Downloads = "1" | ||
| julia = "1.3" | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| Preferences = "1" | ||
| CoolProp_jll = "6.6, 7.1" | ||
|
|
||
| [extras] | ||
| Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" | ||
|
|
||
| [targets] | ||
| test = ["Test"] | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| module CoolPropDevLoader | ||
|
|
||
| using Preferences | ||
| using Downloads | ||
| using CoolProp_jll | ||
|
|
||
| const SOURCEFORGE_URL = "https://sourceforge.net/projects/coolprop/files/CoolProp/nightly/source/CoolProp_sources.zip/download" | ||
|
|
||
| #only available in linux | ||
| function compile_from_sourceforge() | ||
| # 1. Configuration | ||
| url = SOURCEFORGE_URL | ||
| build_dir = joinpath(@__DIR__, "coolprop_build") # Change as needed | ||
| source_zip = joinpath(build_dir, "CoolProp_sources.zip") | ||
| extract_dir = build_dir # We'll extract directly into build_dir, then locate source root | ||
| lib_name = "libCoolProp.so" | ||
|
|
||
| # 2. Create build directory | ||
| mkpath(build_dir) | ||
| mkpath(extract_dir) | ||
|
|
||
| # 3. Download the source zip | ||
| @info "Downloading CoolProp source from $url..." | ||
| Downloads.download(SOURCEFORGE_URL, source_zip) | ||
|
|
||
| # 4. Extract the zip into build_dir | ||
| @info "Extracting source..." | ||
| run(`unzip -q $source_zip -d $build_dir`) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| # 5. Find the actual source root: a directory that contains CMakeLists.txt | ||
| @info "Locating source root..." | ||
| function find_cmakelists_root(dir) | ||
| # Check if dir itself contains CMakeLists.txt | ||
| if isfile(joinpath(dir, "CMakeLists.txt")) | ||
| return dir | ||
| end | ||
| # Otherwise, look for a subdirectory that contains it | ||
| for entry in readdir(dir) | ||
| path = joinpath(dir, entry) | ||
| if isdir(path) && isfile(joinpath(path, "CMakeLists.txt")) | ||
| return path | ||
| end | ||
| end | ||
| # If not found, search recursively (but be careful with deep structures) | ||
| for (root, dirs, files) in walkdir(dir) | ||
| if "CMakeLists.txt" in files | ||
| return root | ||
| end | ||
| end | ||
| error("Could not find CMakeLists.txt anywhere in $dir") | ||
| end | ||
|
|
||
| source_root = find_cmakelists_root(extract_dir) | ||
| @info "Source root found at: $source_root" | ||
|
|
||
| # 6. Prepare for CMake build: create a build subdirectory inside source_root | ||
| build_subdir = joinpath(source_root, "build") | ||
| mkpath(build_subdir) | ||
| cd(build_subdir) do | ||
| # 7. Configure with CMake - build shared library | ||
| @info "Configuring with CMake..." | ||
| run(`cmake .. -DCOOLPROP_SHARED_LIBRARY=ON -DCMAKE_BUILD_TYPE=Release`) | ||
|
|
||
| # 8. Build | ||
| @info "Compiling CoolProp (this may take a few minutes)..." | ||
| run(`cmake --build . --config Release --parallel $(Sys.CPU_THREADS)`) | ||
| end | ||
|
|
||
| # 9. Locate the generated shared library | ||
| # The library is typically in build/ or build/Release/ or build/lib/ | ||
| possible_paths = [ | ||
| joinpath(source_root, "build", lib_name), | ||
| joinpath(source_root, "build", "Release", lib_name), | ||
| joinpath(source_root, "build", "lib", lib_name), | ||
| ] | ||
|
|
||
| lib_path = nothing | ||
| for p in possible_paths | ||
| if isfile(p) | ||
| lib_path = p | ||
| break | ||
| end | ||
| end | ||
|
|
||
| if lib_path === nothing | ||
| error("Could not find compiled library. Looked in: $(join(possible_paths, ", "))") | ||
| end | ||
|
|
||
| @info "CoolProp library built successfully at: $lib_path" | ||
| return lib_path | ||
| end | ||
|
|
||
| struct CompileCoolPropFromSourceforge end | ||
|
|
||
| function use_dev_library(::CompileCoolPropFromSourceforge) | ||
| lib_src = compile_from_sourceforge() | ||
| set_preferences!(CoolProp_jll,"coolprop_library" => lib_src, force = true) | ||
| @info "Preference set to $lib_src" | ||
| end | ||
|
|
||
| function use_dev_library(lib_src::String) | ||
| set_preferences!(CoolProp_jll,"coolprop_library" => lib_src, force = true,active_project_only = false,export_prefs = true) | ||
| @info "Preference set to $lib_src" | ||
| end | ||
|
longemen3000 marked this conversation as resolved.
|
||
|
|
||
| function use_default_library() | ||
| delete_preferences!(CoolProp_jll,"coolprop_library",force = true,export_prefs = true) | ||
| end | ||
|
|
||
| end #module | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add
permissionsblock and setpersist-credentials: falseon checkout.The workflow lacks a
permissionsblock, defaulting to the token's full permissions set. Since this workflow only runs tests, it should use minimal permissions. Additionally,actions/checkoutpersists the GitHub token by default, which is unnecessary here and flagged by zizmor as a credential persistence risk.🔒 Proposed security hardening
name: Development Tests on: schedule: - cron: 0 0 * * * workflow_dispatch: push: paths: - '.github/**' - 'test/CoolPropDevLoader/**' +permissions: + contents: read + concurrency: # Skip intermediate builds: all builds except for builds on the `master` branch # Cancel intermediate builds: only pull request builds group: ${{ github.workflow }}-${{ github.ref }}-${{ github.ref != 'refs/heads/master' || github.run_number }} cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }} jobs: test-coolprop-dev: timeout-minutes: 30 strategy: matrix: julia_version: - '1.12' os: - ubuntu-latest julia_arch: - x64 fail-fast: false runs-on: ${{ matrix.os }} steps: - name: Checkout uses: actions/checkout@v7 + with: + persist-credentials: false📝 Committable suggestion
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 33-34: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 19-58: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Source: Linters/SAST tools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code compiles CoolProp and creates a directory inside the test folder, only read permissions are not enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@longemen3000, good question — but this is a common mix-up between two different types of "permissions."The
permissions:block in a GitHub Actions workflow only controls the scope of theGITHUB_TOKENused for calls to the GitHub API (e.g., pushing commits, creating releases/comments, writing to packages, etc.). It has no effect on local filesystem operations on the runner's disk.Since this workflow's build step (compiling CoolProp via CMake, creating a
builddirectory undertest/, downloading the SourceForge archive, etc.) only writes to the runner's local disk and never pushes back to the repository or calls the GitHub API to write content,contents: readis sufficient. The job can still freely create directories, compile code, and download files locally — that's unrelated to the token's API scope.So
contents: readwon't block the compile step; it just prevents theGITHUB_TOKENfrom being used to push/write to the repo via the API, which this workflow doesn't do anyway. Let me know if you're hitting an actual permission-denied error from a specific step, and I can help dig into that separately.✏️ Learnings added