Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Remove generation of INCLUDE file in the tests#1295

Merged
1uc merged 2 commits into
masterfrom
pramodk/parallel-test
Jun 4, 2024
Merged

Remove generation of INCLUDE file in the tests#1295
1uc merged 2 commits into
masterfrom
pramodk/parallel-test

Conversation

@pramodk

@pramodk pramodk commented Jun 3, 2024

Copy link
Copy Markdown
Contributor
  • include mod files generation cause race condition with parallel tests, see Fix ctest --parallel #1290
  • the included files are trivial examples and they don't need to be generated from the code
    • could be copied via configure_file
    • cmake could generate those at a configure time

Considering the use case and it's triviality, I thought 2nd option is better. That also allows removal of some extra code/logic.

- include mod files generation cause race condition with parallel tests, see #1290
- the included files are trivial examples for testing and they don't need to be
  generated from the code
  - could be copied via configure_file
  - cmake could generate those at a configure time

Considering the use case and it's triviality, I thought 2nd option is better. That
also allows to remove some extra code/logic.
@pramodk pramodk requested a review from 1uc June 3, 2024 19:45
@bbpbuildbot

This comment has been minimized.

bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Jun 4, 2024
@codecov

codecov Bot commented Jun 4, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.49%. Comparing base (5366532) to head (e593a93).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1295   +/-   ##
=======================================
  Coverage   85.49%   85.49%           
=======================================
  Files         178      178           
  Lines       13448    13436   -12     
=======================================
- Hits        11497    11487   -10     
+ Misses       1951     1949    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bbpbuildbot

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #214695 (:white_check_mark:) have been uploaded here!

Status and direct links:

@1uc 1uc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It ran several times without issues.

@1uc 1uc merged commit 844e6bd into master Jun 4, 2024
@1uc 1uc deleted the pramodk/parallel-test branch June 4, 2024 12:07
@JCGoran JCGoran mentioned this pull request Jun 5, 2024
JCGoran pushed a commit to neuronsimulator/nrn that referenced this pull request Mar 12, 2025
* Remove INCLUDE mod file generation in tests

- include mod files generation cause race condition with parallel tests, see BlueBrain/nmodl#1290
- the included files are trivial examples for testing and they don't need to be
  generated from the code
  - could be copied via configure_file
  - cmake could generate those at a configure time

Considering the use case and it's triviality, I thought 2nd option is better. That
also allows to remove some extra code/logic.

NMODL Repo SHA: BlueBrain/nmodl@844e6bd
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants