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.

Sign up for GitHub

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

Updated CI #25

Merged
merged 10 commits into from
Mar 26, 2020
Merged

Updated CI #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
18a52d4
Update .travis.yml
ewu63 Mar 23, 2020
2b6e5bc
Update .travis.yml
ewu63 Mar 23, 2020
e8e90cf
Removed py2
ewu63 Mar 23, 2020
f198d39
Removed pip cache
ewu63 Mar 24, 2020
aa15119
Update .travis.yml
ewu63 Mar 24, 2020
a43fbe6
Added back the example builds as tests
ewu63 Mar 26, 2020
f467663
added tests for building the examples
ewu63 Mar 26, 2020
e1e45a5
Merge branch 'master' into travis-test
ewu63 Mar 26, 2020
565dce7
Update .travis.yml
ewu63 Mar 26, 2020
47fd876
Update .travis.yml
ewu63 Mar 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 7 additions & 25 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ os:
language: generic

env:
- PY=2.7
- PY=3.6
- PY=3.7
- PY=3.8

addons:
apt:
Expand All @@ -17,7 +18,6 @@ addons:
cache:
apt: true
directories:
- $HOME/.cache/pip
- $HOME/miniconda

before_install:
Expand All @@ -33,18 +33,9 @@ before_install:
rm -rf $HOME/miniconda;
fi

- if [ -f $HOME/.cache/pip ]; then
echo "cached pip found -- nothing to do";
else
NOT_CACHED_PIP=1;
rm -rf $HOME/.cache/pip;
fi

- rm -rf $HOME/miniconda/lib/python$PY/site-packages/pyXDSM

install:
- if [ "$NOT_CACHED_CONDA" ]; then
wget "https://repo.continuum.io/miniconda/Miniconda-latest-Linux-x86_64.sh" -O miniconda.sh;
wget "https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh" -O miniconda.sh;
chmod +x miniconda.sh;
./miniconda.sh -b -p $HOME/miniconda;
export PATH=$HOME/miniconda/bin:$PATH;
Expand All @@ -54,23 +45,14 @@ install:
export PATH=$HOME/miniconda/bin:$PATH;
fi

- if [ "$NOT_CACHED_PIP" ]; then
pip install --upgrade pip;
fi

- pip install .

- pip install testflo;
- pip install -e .

# display summary of installed packages and their versions
- conda list

script:
- cd examples
- python kitchen_sink.py
- python mat_eqn.py
- python mdf.py
- ls
- cd ..
- testflo . -v

deploy:
provider: pypi
Expand All @@ -79,4 +61,4 @@ deploy:
distributions: "sdist bdist_wheel"
on:
tags: true
skip_existing: true
skip_existing: true
20 changes: 18 additions & 2 deletions pyxdsm/tests/test_xdsm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import unittest
import os

from pyxdsm.XDSM import XDSM
from pyxdsm.XDSM import XDSM, __file__


class TestXDSM(unittest.TestCase):
Expand All @@ -26,6 +25,23 @@ def tearDown(self):
except OSError:
pass

def test_examples(self):
'''
This test just builds the three examples, and assert that the output files exist.
Unlike the other tests, this one requires LaTeX to be available.
'''
os.chdir(os.path.join(os.path.dirname(os.path.abspath(__file__)), '../examples'))
filenames = ['kitchen_sink', 'mdf']
for f in filenames:
os.system('python {}.py'.format(f))
self.assertTrue(os.path.isfile(f + '.tikz'))
self.assertTrue(os.path.isfile(f + '.tex'))
self.assertTrue(os.path.isfile(f + '.pdf'))
Copy link
Contributor

@onodip onodip Mar 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think this should only be asserted, if pdflatex is installed, otherwise it will fail.
Like here: https://github.com/onodip/OpenMDAO-XDSM/blob/master/omxdsm/tests/test_xdsm_viewer.py#L167

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Really good point, will borrow your approach from the XDSM plugin, thanks for pointing this out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm should the logic be changed to:
self.assertTrue(not pdflatex or (pdflatex and os.path.isfile(f + '.pdf')))?
The way it's done in the referenced repo would mean that if pdflatex is installed, then it doesn't actually check isfile right?

Copy link
Contributor

@onodip onodip Mar 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Should be equivalent. Either not pdflatex is True, in this case the statement after the or is not even evaluated (since the PDF cannot be possibly created, it is not an error, if the file does not exist, so this is okay). If not pdflatex is False - so pdflatex is installed, then it checks the file. In your line the or is only reached, if pdflatex is True, so it does not have to be checked again, that pdflatex is True.

I added below a snippet, which shows, that the two approaches give the same result in all combinations. s3 is there to clarify the precedence. a=pdflatex, b=os.path.isfile(f + '.pdf')

for a in (True, False):
    for b in (True, False):
        print(f'{a=}')
        print(f'{b=}')
        s1 = not a or b
        s2 = not a or (a and b)
        s3 = (not a) or b
        print(f'{s1=}')
        print(f'{s2=}')
        print(f'{s3=}')
        print('\n')

a=True
b=True
s1=True
s2=True
s3=True

a=True
b=False
s1=False
s2=False
s3=False

a=False
b=True
s1=True
s2=True
s3=True

a=False
b=False
s1=True
s2=True
s3=True

Note: I found another minor logical error in the code. If pdflatex is not installed, and the file is there, the assertion is okay. This could be suspicious (maybe the file was there before the test, and therefore the test would always pass), but since a temporarily directory is used for the tests, that cannot be the case. The totally correct way would be (a and b) or ((not a) and (not b))

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right that makes sense, thanks for that. Cheers!

os.system('python mat_eqn.py')
self.assertTrue(os.path.isfile('mat_eqn_example.pdf'))
# change back to previous directory
os.chdir(self.tempdir)

def test_options(self):

filename = 'xdsm_test_options'
Expand Down