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

Move old tests #36

Merged
merged 31 commits into from
Jul 4, 2023
Merged

Move old tests #36

merged 31 commits into from
Jul 4, 2023

Conversation

stewartboogert
Copy link
Member

Moved most of the tests over

  • All input files are from g4testdata
  • Some fixtures need work.
  • Approximately 600/620 passing

@gipert
Copy link
Member

gipert commented Jul 2, 2023

There is a pre-commit check called "python tests naming" that complains about test files not being named test_*.py. I admit it might be too pedantic so feel free to disable it, if you feel like.

@gipert
Copy link
Member

gipert commented Jul 2, 2023

So this fixture here:

@pytest.fixture(scope="session")
def tmptestdir():
_tmptestdir.mkdir()
return _tmptestdir
def pytest_sessionfinish(session, exitstatus):
if exitstatus == 0 and _tmptestdir.exists():
shutil.rmtree(_tmptestdir)

used, for example, here:

Writer.writeVtkPolyDataAsSTLFile(str(tmptestdir / "T001_Box.stl"), [pd])

sets up a random directory in /tmp for files generated by the test suite and auto-destroys if pytest is successful. The motivation for this is to avoid polluting the local clone with these files and clean them up after every run. It would be ideal to use it everywhere...

@gipert
Copy link
Member

gipert commented Jul 2, 2023

Non-Python files in features/ have been committed by mistake, I presume?

@gipert
Copy link
Member

gipert commented Jul 2, 2023

I see you depend on PyROOT for some tests, which is not available in CI. We should discuss how to install this dependency, if it's really necessary...

@stewartboogert
Copy link
Member Author

stewartboogert commented Jul 3, 2023

There is a pre-commit check called "python tests naming" that complains about test files not being named test_*.py. I admit it might be too pedantic so feel free to disable it, if you feel like.

I was going to ask. Fixtures is some cases is an excellent solution , but it does making running some code outside of purest cumbersome. I'll disable for now

@stewartboogert
Copy link
Member Author

stewartboogert commented Jul 3, 2023

I see you depend on PyROOT for some tests, which is not available in CI. We should discuss how to install this dependency, if it's really necessary...

There is a converter from root and it would be great to test against. A standard root install in the docker image would be outstanding. Does not need to be anything magical, just working with the python we use.

Should I disable them for now?

@stewartboogert
Copy link
Member Author

So this fixture here:

@pytest.fixture(scope="session")
def tmptestdir():
_tmptestdir.mkdir()
return _tmptestdir
def pytest_sessionfinish(session, exitstatus):
if exitstatus == 0 and _tmptestdir.exists():
shutil.rmtree(_tmptestdir)

used, for example, here:

Writer.writeVtkPolyDataAsSTLFile(str(tmptestdir / "T001_Box.stl"), [pd])

sets up a random directory in /tmp for files generated by the test suite and auto-destroys if pytest is successful. The motivation for this is to avoid polluting the local clone with these files and clean them up after every run. It would be ideal to use it everywhere...

Exellent I was going to ask how to do this. I am currently littering the directory structure with files which I want to suppress.

@gipert
Copy link
Member

gipert commented Jul 3, 2023

There is a converter from root and it would be great to test against. A standard root install in the docker image would be outstanding. Does not need to be anything magical, just working with the python we use.

Should I disable them for now?

Yes, please. Could you open an issue about PyROOT to keep track of it?

@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #36 (79dd648) into main (e3f8e29) will increase coverage by 44.36%.
The diff coverage is 61.11%.

@@             Coverage Diff             @@
##             main      #36       +/-   ##
===========================================
+ Coverage   24.24%   68.61%   +44.36%     
===========================================
  Files         144      144               
  Lines       21024    21036       +12     
===========================================
+ Hits         5098    14434     +9336     
+ Misses      15926     6602     -9324     
Impacted Files Coverage Δ
src/pyg4ometry/geant4/Registry.py 66.60% <46.15%> (+41.79%) ⬆️
src/pyg4ometry/fluka/body.py 84.63% <100.00%> (+57.79%) ⬆️
src/pyg4ometry/fluka/preprocessor.py 83.43% <100.00%> (+64.33%) ⬆️
...c/pyg4ometry/visualisation/VisualisationOptions.py 83.87% <100.00%> (+51.61%) ⬆️

... and 82 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stewartboogert stewartboogert merged commit 53585c4 into g4edge:main Jul 4, 2023
13 checks passed
@stewartboogert stewartboogert deleted the tests branch July 4, 2023 18:48
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.

None yet

2 participants