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

Allow NELM and length as aliases for waveform length #79

Merged
merged 4 commits into from
Feb 7, 2022

Conversation

Araneidae
Copy link
Collaborator

Fixes issue #37

@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #79 (0115f40) into master (53f107d) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   88.47%   88.57%   +0.10%     
==========================================
  Files          13       13              
  Lines         902      910       +8     
==========================================
+ Hits          798      806       +8     
  Misses        104      104              
Impacted Files Coverage Δ
softioc/builder.py 97.08% <100.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53f107d...0115f40. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Unit Test Results

     12 files  ±  0       12 suites  ±0   14m 41s ⏱️ +6s
   221 tests +  7     214 ✔️ +  7      7 💤 ±0  0 ±0 
2 644 runs  +84  2 292 ✔️ +84  352 💤 ±0  0 ±0 

Results for commit 0115f40. ± Comparison against base commit 53f107d.

This pull request removes 2 and adds 9 tests. Note that renamed tests count towards both.
tests.test_records ‑ test_waveform_disallows_too_long_values
tests.test_records ‑ test_waveform_disallows_zero_length
tests.test_record_values.TestGetValue ‑ test_value_pre_init_initial_value[stringIn-str-a 39 char string exactly maximum length]
tests.test_record_values.TestGetValue ‑ test_value_pre_init_initial_value[stringOut-str-a 39 char string exactly maximum length]
tests.test_record_values.TestGetValue ‑ test_value_pre_init_set[stringIn-str-a 39 char string exactly maximum length]
tests.test_record_values.TestGetValue ‑ test_value_pre_init_set[stringOut-str-a 39 char string exactly maximum length]
tests.test_record_values.TestInvalidValues ‑ test_long_string_rejects_overlong_strings
tests.test_record_values.TestInvalidValues ‑ test_string_rejects_overlong_strings
tests.test_record_values.TestInvalidValues ‑ test_waveform_rejects_overlong_values
tests.test_record_values.TestInvalidValues ‑ test_waveform_rejects_zero_length
tests.test_records ‑ test_waveform_construction

♻️ This comment has been updated with latest results.

@AlexanderWells-diamond
Copy link
Collaborator

AlexanderWells-diamond commented Feb 3, 2022

@Araneidae I think this is done now, I added two separate sets of tests in my two commits.

I will note that I haven't managed to explain the failures I saw on CI, and the fixes I did for them were little more than adding a few more assert statements to check some return codes. Simply adding these made the failures disappear for no reason I can fathom. I'm slightly concerned that there's some underlying problem with the entire test framework I've written, perhaps something in cothread or pythonSoftIOC itself that really doesn't like being used alongside multiprocessing. Exactly what it may be I can't say unfortunately.

Happy for you to merge this whenever/however you think best.

EDIT: I tweaked (and force-pushed) my last commit to fix some documentation and naming convention issues, but no actual code changes, and that somehow caused the tests to start failing again. I guess this warrants additional investigation...

Other fails appear to be the spawned Validate Process not correctly
updating, or possibly not being removed from previous test run, on each
iteration. Workaround is to give each one a unique device prefix and
add logging to work out what it might be for future issues.
These tests have passed 10 consecutive runs on CI with no failures.

There were race conditions regarding processing of record callbacks, so
we swap to a model of using a "DONE" record that, when processed,
signals that the IOC process has finished all of its callbacks.

Logging also added to some tests that had weird failures on CI, so if
they appear again we'll have more information.
@AlexanderWells-diamond
Copy link
Collaborator

@Araneidae I think I've finished test fixes. My last commit has passed 10 consecutive times on the test_fix branch's CI.

Let me know when/how you want this merged back to master.

@Araneidae
Copy link
Collaborator Author

Looks like a clean commit chain that solves a lot of problems.

Go for it, and think we can, finally, go for a release!

@AlexanderWells-diamond AlexanderWells-diamond merged commit 8e64a7b into master Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

String record handling data truncation Waveform records ignore NELM keyword if initial_value is set
2 participants