-
Notifications
You must be signed in to change notification settings - Fork 2
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 fourfront from pytest 2.9 to 3.10 (ALT) #1437
base: master
Are you sure you want to change the base?
Conversation
…tilities level. (Many are unused but may back other code we want to port.) Rename src/encoded/utils.py to src/encoded/util.py so these filenames will align again. Adjust files that import the renamed file. Also adjust test_indexing.py to use meta = MetaData(...); meta.reflect() instead of meta = MetaData(..., reflect=True).
…kibana-start-test and use poetry 1.1.4 in Makefile.
…ile.ZipFile) from cgap-portal version of scr/encoded/xlreader.py
… beause they'd be complicated to fix right now.
…src/encoded/tests/test_access_key.py
…on't special-case GET and HEAD methods.
…iverges. Add a TEST_PREFIX variable for symmetry. Compute the letter pairs needed for accession ids by looking at the schema files.
…CCEPT, which is more like what browsers would do and is needed for some ported changes from cgap-portal. Light PEP8. Remove pytest_plugins setting, which is deprecated; this info is now in pytest.ini. Change pytest.ini to have markers in line with cgap-portal, to use testpaths rather than a --pyargs setting, and to use -p to handle plugins.
…/renderers.py. Some PEP8.
…nge the way commits work to be more incremental.
…broken everywhere, not just on remote testing.
…data showthrough. Fix a backslash bug in test_types_init_collections.py
…test_static_page.py
…for schema_formats.py that check what's declared against what's needed.
travis-test: # Actually, we don't normally use this. Instead the GA workflow sets up two parallel tests. | ||
make travis-test-npm | ||
make travis-test-unit | ||
|
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.
Some of the changes in this file, and this PR in general, are just leveling with cgap-portal
to minimize dissonance. But here we also added some criteria for what to use for make
.
Note that in spite of all these markers, I still had to mark tests not to run with skip
since otherwise if I try to run the file by name with, for example, bin/test -vv -k test_search
it wouldn't confuse me by running things marked broken because I hadn't in my by-hand invocation listed all relevant markers.
The sloppy
test is presently marking tests that break stuff because they don't manage side-effects well. It's possible that if they were divvied out differently, like with the indexing tests, it would be OK.
--pyargs encoded.tests deploy.tests | ||
-p encoded.tests deploy.tests | ||
-p encoded.tests.datafixtures | ||
-p snovault.tests.serverfixtures | ||
--instafail |
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 above changes to addopts
are what make pycharm
work. It will invoke things with pytest
, not bin/test
and it's critical that all the options that are needed for it to run are correctly specified. In the old form, it worked to add a -k <something>
on the command line to run a filtered test, but there was no way to run a single test because it was always going to run encoded.tests
and deploy.tests
no matter what you did. In the new way, you aren't forcing any tests to run by default. They are chosen as "all" if you specify no specific tests, and they're filtered against the directories specified later in the file (see testpaths
below.
Pytest 3.10 also somewhere in there requires that you set the plugins information here on the command line (with -p
) rather than inline in conftest.py
files by setting pytest_plugins
. It wants all that plugin stuff done at startup time, I think so that all the session fixtures go in those files and so it is known at startup which session things are needed to have session scope so it can activate them on first use reliably.
--pyargs encoded.tests deploy.tests | ||
-p encoded.tests deploy.tests | ||
-p encoded.tests.datafixtures | ||
-p snovault.tests.serverfixtures | ||
--instafail | ||
markers = |
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.
I imported all the cgap marker names here. They don't create overhead and it makes it easier to copy code back and forth.
@@ -1,5 +1,60 @@ | |||
#!/bin/bash | |||
|
|||
docker_kibana_image=docker.elastic.co/kibana/kibana-oss:6.8.9 |
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.
This file is just copied from cgap-portal
. No code changes in this PR.
@@ -0,0 +1,70 @@ | |||
#!/bin/bash |
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.
Also just copied from cgap-portal
without changes.
assert 'Accession and uuid are automatically assigned during initial posting' in res.json['content'][0]['content'] | ||
assert res.json['toc'] == posted_help_page['table-of-contents'] | ||
|
||
|
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.
I added this test.
assert 'Accession and uuid are automatically assigned during initial posting' in res.json['content'][0]['content'] | ||
assert res.json['toc'] == posted_help_page['table-of-contents'] | ||
|
||
|
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.
I added this test.
|
||
|
||
@pytest.fixture(scope='module') | ||
def help_page_deleted(testapp, posted_help_page_section, help_page_json_draft): |
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.
Note that these fixtures were kind of a mess, with the draft json going into the deleted page, etc. I tried to clean that kind of thing up. It may be worth looking at the new file in direct mode, not side-by-side.
pytest.mark.schema, | ||
pytest.mark.indexing, | ||
#pytest.mark.flaky(rerun_filter=customized_delay_rerun(sleep_seconds=10)) | ||
] | ||
|
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.
Note: not an indexing test. It uses workbook.
def check_item_type(client, item_type): | ||
# This might get a 404 if not enough time has elapsed, so try a few times before giving up. | ||
return client.get('/%s?limit=all' % item_type, status=[200, 301]).follow() | ||
class ItemTypeChecker: |
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.
This is ported in from cgap-portal.
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.
I've added some small comments but the vast majority of these changes look reasonable to me and are unlikely to cause issues - I would consider deploying this somewhere to be sure prior to merging.
@@ -38,7 +40,7 @@ macpoetry-install: # Same as 'poetry install' except that on OSX Catalina, an e | |||
|
|||
configure: # does any pre-requisite installs | |||
pip install --upgrade pip | |||
pip install poetry==1.0.10 # pinned to avoid build problems we cannot fix in pyproject.toml | |||
pip install poetry==1.1.4 # poetry latest as of 1/25/2021 seemed to work but apparantly does not |
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.
This comment is actually out of date now, 1.1.4
is latest and didn't work without the fix-dist-info step, which we now have so we're good.
# TODO: CGAP does this differently. If port 443 is used, Will says use_ssl is implied and can be omitted here. | ||
# Need to verify that and then make both systems agree here. -kmp 16-Feb-2021 |
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.
Port 443
is specified by the server URL, so it is not needed here as well.
"indexer": settings.get("indexer"), | ||
"index_server": settings.get("index_server"), |
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.
Shouldn't this have stayed the same?
|
||
pytestmark = [pytest.mark.indexing, pytest.mark.flaky(rerun_filter=delay_rerun)] | ||
pytestmark = [pytest.mark.flaky(rerun_filter=delay_rerun), pytest.mark.triage] |
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.
I think the answer to this is "not working".
Substantive changes and bug fixes:
In
src/encoded/authentication.py
GET
andHEAD
methods.pyproject.toml
pytest
to version 3.10 and related libraries in some ways.awscli
,boto3
,botocore
,netaddr
,boto3-stubs
.In
src/encoded/schema_formats.py
andsrc/encoded/server_defaults.py
:ACCESSION_PREFIX
variable to minimize number of places code diverges.TEST_PREFIX
variable for symmetry. Compute the letter pairs needed for accession ids by looking at the schema files.src/encoded/loadxl.py
post_only=
argument forload_all
andload_all_gen
.src/encoded/renderers.py
src/encoded/tests/conftest.py
htmltestapp
andanonhtmltestapp
to passHTTP_ACCEPT
, which is more like what browsers would do, and which is needed for changes ported from CGAP.pytest_plugins
setting, which is deprecated. This info is now inpytest.ini
.pytest.ini
testpaths
rather than a--pyargs
argument being added.-p
to handle plugins, in lieu of settingpytest_plugins
.src/encoded/tests/test_indexing.py
meta = MetaData(...); meta.reflect()
to instead bemeta = MetaData(..., reflect=True)
.Makefile
:scripts/kibana-start
andscripts/psql-start
scripts to support targetskibana-start-test
andpsql-test
, respectively.poetry 1.1.4
conftest.py
tempfile.tempdir
to'/tmp'
.src/encoded/commands/purge_item_type.py
Migrate the es_testapps to
src/encoded/tests/conftest.py
fromsrc/encoded/tests/workbook.py
,and update the following tests to use that test_app:
test_validation_errors.py
test_purge_item_type.py
test_static_page.py
test_indexing.py
test_aggregation.py
test_batch_download.py
Adjust
Makefile
targettest
to split indexing and non-indexing cases the way CGAP does.Simple changes and small bug fixes (mostly found by better warnings):
Mark
test_index_data_workbook
as skip in test_search.py.Use
str
in place ofbasestring
insrc/encoded/loadxl.py
andsrc/encoded/tests/test_loadxl.py
.Port new file
src/encoded/tests/test_renderers.py
from CGAP.src/encoded/tests/test_static_page.py
src/encoded/types/page.py
src/encoded/tests/test_types_biosample.py
biosample_relation
as a fixture. It's just a function.Add support for rewriting
"~1.2.3"
insetup_eb.py
. Add test cases intest_setup_eb.py
.src/encoded/commands/spreadsheet_to_json.py
argparse
.src/encoded/xlreader.py
zipfile.ZipFile
).Relock
poetry.lock
.Rename
src/encoded/utils.py
back tosrc/encoded/util.py
for alignment with cgap-portal, and with history. Bad idea to have renamed it in the first place.Port various utilities that might not be used, just in case they're needed for other ports. We can clean this up later.
Update callers to import from
util
:src/encoded/types/file.py
src/encoded/tests/test_search.py
src/encoded/tests/test_types_init_collections.py
src/encoded/tests/test_types_protocol.py
src/encoded/tests/test_util.py
(wastest_utils.py
, but renamed as well)src/encoded/tests/test_validation_errors.py
src/encoded/tests/test_aggregation.py
src/encoded/tests/test_batch_download.py
Move the definition or
ORDER
to `conftest_settiGet rid of unused
config
fixture onsrc/encoded/tests/conftest.py
.Get rid of unused
moto
fixture onsrc/encoded/tests/conftest.py
.Cosmetic changes:
Change
.encrypt
to.hash
insrc/encoded/tests/test_access_key.py
src/encoded/types/access_key.py
src/encoded/tests/test_edw_hash.py
Use
dcicutils.misc_utils.TestApp
instead ofwebtest.TestApp
in:src/encoded/commands/import_data.py
src/encoded/commands/load_access_keys.py
.ebextensions/13_run_npm.config
.gitignore
src/encoded/commands/configure_kibana_index.py
{'use_ssl': True}
.src/encoded/dev_servers.py
Also misc PEP8 in various files, including:
src/encoded/root.py