diff --git a/.ci/autopep8.sh b/.ci/autopep8.sh deleted file mode 100755 index 22c385c425b..00000000000 --- a/.ci/autopep8.sh +++ /dev/null @@ -1,2 +0,0 @@ -exclude=$(sed -e 's|^|./|' -e 's|/$||' .ci/flake8_ignorelist.txt | paste -s -d ',' - ) -autopep8 -i -r --exclude $exclude --select E11,E101,E127,E201,E202,E22,E301,E302,E303,E304,E306,E711,W291,W292,W293,W391 ./lib/ ./test/ diff --git a/.ci/check_py3_compatibility.sh b/.ci/check_py3_compatibility.sh deleted file mode 100755 index e275c0d1b68..00000000000 --- a/.ci/check_py3_compatibility.sh +++ /dev/null @@ -1,35 +0,0 @@ -#!/bin/sh - -if command -v ack-grep >/dev/null; then - ACK=ack-grep -else - ACK=ack -fi - - -PYTHON2_ONLY_MODULES="__builtin__ _winreg BaseHTTPServer CGIHTTPServer \ -ConfigParser Cookie cookielib copy_reg cPickle cStringIO Dialog dummy_thread \ -FileDialog gdbm htmlentitydefs HTMLParser httplib Queue robotparser \ -ScrolledText SimpleDialog SimpleHTTPServer SimpleXMLRPCServer SocketServer \ -StringIO thread Tix tkColorChooser tkCommonDialog Tkconstants Tkdnd tkFont \ -Tkinter tkFileDialog tkMessageBox tkSimpleDialog ttk urllib urllib2 urlparse \ -xmlrpclib" - -ret=0 -for mod in $PYTHON2_ONLY_MODULES; do - $ACK --type python \ - --ignore-dir=.git \ - --ignore-dir=.tox \ - --ignore-dir=.venv \ - --ignore-dir=.venv3 \ - --ignore-dir=client/node_modules \ - --ignore-dir=database \ - --ignore-dir=doc/build \ - --ignore-dir=eggs \ - --ignore-dir=static/maps \ - --ignore-dir=static/scripts \ - "^import $mod(\n|\.)|^from $mod import " - if [ $? -eq 0 ]; then ret=1; fi -done - -exit $ret diff --git a/.ci/check_test_class_names.sh b/.ci/check_test_class_names.sh new file mode 100755 index 00000000000..76bf70057a3 --- /dev/null +++ b/.ci/check_test_class_names.sh @@ -0,0 +1,7 @@ +#!/bin/sh +n_tests=$(pytest --collect-only --ignore=test/functional lib/galaxy_test/ test/ | grep 'tests collected' | sed -e 's/[^0-9]*\([0-9]*\) tests collected.*/\1/') +n_tests_extra_classes=$(pytest -o python_classes='Test* *Test *TestCase' --collect-only --ignore=test/functional lib/galaxy_test/ test/ | grep 'tests collected' | sed -e 's/[^0-9]*\([0-9]*\) tests collected.*/\1/') +if [ "$n_tests_extra_classes" -gt "$n_tests" ]; then + echo "New test class with name not starting with Test introduced, change it to have tests collected by pytest" + exit 1 +fi diff --git a/.github/workflows/check_test_class_names.yaml b/.github/workflows/check_test_class_names.yaml new file mode 100644 index 00000000000..92db607c7bd --- /dev/null +++ b/.github/workflows/check_test_class_names.yaml @@ -0,0 +1,28 @@ +name: Check test class names +on: + pull_request: + paths: + - '.ci/check_test_class_names.sh' + - 'lib/galaxy_test/**' + - 'test/**' +jobs: + test: + name: Test + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ['3.7'] + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + - name: Cache pip dir + uses: actions/cache@v3 + with: + path: ~/.cache/pip + key: pip-cache-${{ matrix.python-version }}-${{ hashFiles('requirements.txt') }} + - name: Install Python dependencies + run: pip install -r requirements.txt -r lib/galaxy/dependencies/dev-requirements.txt + - name: Run tests + run: .ci/check_test_class_names.sh diff --git a/.github/workflows/converter_tests.yaml b/.github/workflows/converter_tests.yaml index dc3179f8b42..f9e6649f86e 100644 --- a/.github/workflows/converter_tests.yaml +++ b/.github/workflows/converter_tests.yaml @@ -59,24 +59,11 @@ jobs: - name: Lint converters run: | mapfile -t TOOL_ARRAY < tool_list.txt - for CONVERTER in "${TOOL_ARRAY[@]}"; do - planemo lint --skip citations,stdio,help --report_level warn "$CONVERTER" - done + planemo lint --skip citations,stdio,help --report_level warn "${TOOL_ARRAY[@]}" - name: Run tests run: | - mkdir -p json_output - EXIT_CODE=0 mapfile -t TOOL_ARRAY < tool_list.txt - for CONVERTER in "${TOOL_ARRAY[@]}"; do - json=$(mktemp -u -p json_output --suff .json) - planemo test --test_output_json "$json" --galaxy_python_version ${{ matrix.python-version }} --galaxy_root 'galaxy root' "$CONVERTER" || EXIT_CODE=$? - done - planemo merge_test_reports json_output/*.json tool_test_output.json - planemo test_reports tool_test_output.json --test_output tool_test_output.html - if [ "$EXIT_CODE" -ne 0 ]; then - echo "Unsuccessful tests found, inspect the 'Converter test results' artifact for details." - exit $EXIT_CODE - fi + planemo test --galaxy_python_version ${{ matrix.python-version }} --galaxy_root 'galaxy root' "${TOOL_ARRAY[@]}" - uses: actions/upload-artifact@v3 if: failure() with: diff --git a/.github/workflows/mulled.yaml b/.github/workflows/mulled.yaml index f3cf5b3d881..a9e84ef28a5 100644 --- a/.github/workflows/mulled.yaml +++ b/.github/workflows/mulled.yaml @@ -18,6 +18,7 @@ jobs: name: Test runs-on: ubuntu-latest strategy: + fail-fast: false matrix: python-version: ['3.7'] steps: @@ -33,7 +34,6 @@ jobs: run: echo "version=$(python -c 'import sys; print("-".join(str(v) for v in sys.version_info))')" >> $GITHUB_OUTPUT - name: Cache pip dir uses: actions/cache@v3 - id: pip-cache with: path: ~/.cache/pip key: pip-cache-${{ matrix.python-version }}-${{ hashFiles('galaxy root/requirements.txt') }} @@ -42,9 +42,11 @@ jobs: with: path: .tox key: tox-cache-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-mulled + - name: Install Apptainer's singularity + uses: eWaterCycle/setup-apptainer@v2 - name: Install tox run: pip install tox - - name: run tests + - name: Run tests run: tox -e mulled working-directory: 'galaxy root' - uses: actions/upload-artifact@v3 diff --git a/.github/workflows/unit.yaml b/.github/workflows/unit.yaml index 677cfdaa961..cef323f1d2c 100644 --- a/.github/workflows/unit.yaml +++ b/.github/workflows/unit.yaml @@ -37,15 +37,17 @@ jobs: with: path: ~/.cache/pip key: pip-cache-${{ matrix.python-version }}-${{ hashFiles('galaxy root/requirements.txt') }} - - name: Cache galaxy venv + - name: Cache tox env uses: actions/cache@v3 with: - path: 'galaxy root/.venv' - key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-unit + path: .tox + key: tox-cache-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-unit - name: Install ffmpeg run: sudo apt-get update && sudo apt-get -y install ffmpeg + - name: Install tox + run: pip install tox - name: Run tests - run: ./run_tests.sh --coverage -u + run: tox -e unit-coverage working-directory: 'galaxy root' - uses: codecov/codecov-action@v3 with: diff --git a/.pre-commit-config.yaml.sample b/.pre-commit-config.yaml.sample index 58c439ab451..0523e68b212 100644 --- a/.pre-commit-config.yaml.sample +++ b/.pre-commit-config.yaml.sample @@ -1,11 +1,10 @@ repos: - repo: https://github.com/psf/black - rev: 22.1.0 + rev: 22.10.0 hooks: - id: black - language_version: python3.7 - - repo: https://gitlab.com/pycqa/flake8 - rev: 4.0.1 + - repo: https://github.com/pycqa/flake8 + rev: 5.0.4 hooks: - id: flake8 - repo: https://github.com/pre-commit/mirrors-prettier @@ -15,20 +14,21 @@ repos: types: [file] types_or: [javascript, jsx, ts, tsx, vue] - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.1.0 # Use the ref you want to point at + rev: v4.3.0 # Use the ref you want to point at hooks: - id: trailing-whitespace - id: check-merge-conflict - id: check-symlinks - id: destroyed-symlinks - id: end-of-file-fixer + - id: name-tests-test - repo: https://github.com/detailyang/pre-commit-shell - rev: v1.0.6 + rev: 1.0.5 hooks: - id: shell-lint args: [--format=json] - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.14.0 + rev: 0.19.2 hooks: - id: check-github-workflows - repo: local diff --git a/Makefile b/Makefile index 1d953e0a959..ca88e90de8a 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ VENV?=.venv # Source virtualenv to execute command (flake8, sphinx, twine, etc...) IN_VENV=if [ -f "$(VENV)/bin/activate" ]; then . "$(VENV)/bin/activate"; fi; -RELEASE_CURR:=22.09 +RELEASE_CURR:=23.0 RELEASE_UPSTREAM:=upstream CONFIG_MANAGE=$(IN_VENV) python lib/galaxy/config/config_manage.py PROJECT_URL?=https://github.com/galaxyproject/galaxy @@ -47,9 +47,6 @@ format: ## Format Python code base remove-unused-imports: ## Remove unused imports in Python code base $(IN_VENV) autoflake --in-place --remove-all-unused-imports --recursive --verbose lib/ test/ -list-dependency-updates: setup-venv - $(IN_VENV) pip list --outdated --format=columns - docs-slides-ready: test -f plantuml.jar || wget http://jaist.dl.sourceforge.net/project/plantuml/plantuml.jar java -jar plantuml.jar -c $(DOC_SOURCE_DIR)/slideshow/architecture/images/plantuml_options.txt -tsvg $(SLIDESHOW_DIR)/architecture/images/ *.plantuml.txt diff --git a/client/.eslintrc.json b/client/.eslintrc.json index 5df4f328d4a..9d3637dcf86 100644 --- a/client/.eslintrc.json +++ b/client/.eslintrc.json @@ -24,16 +24,20 @@ "vue/valid-v-slot": "error", "vue/v-slot-style": ["error", { "atComponent": "v-slot", "default": "v-slot", "named": "longform" }], - // vue/multi-word-component names is considered an error, that we - // downgrade to a warning here. It should be prioritized to get fixed - // and compliant. - // TODO: We want to fix or add inline exceptions documenting these. + // Downgrade the severity of some rules to warnings as a transition measure. + // For example, vue/multi-word-component names is considered an error, + // but that kind of refactoring is best done slowly, one bit at a time + // as those components are touched. "vue/multi-word-component-names": "warn", "vue/prop-name-casing": "warn", "vue/require-prop-types": "warn", "vue/require-default-prop": "warn", "vue/no-v-html": "warn", + // Increase the severity of some rules to errors + "vue/attributes-order": "error", + "vue/order-in-components": "error", + // Prettier compromises/workarounds -- mostly #wontfix? "vue/html-indent": "off", "vue/max-attributes-per-line": "off", @@ -54,6 +58,29 @@ "vuejs-accessibility/mouse-events-have-key-events": "warn", "vuejs-accessibility/no-autofocus": "error", "vuejs-accessibility/tabindex-no-positive": "error" + }, - "ignorePatterns": ["src/qunit", "src/mocha", "src/libs", "src/nls", "src/legacy"] + "ignorePatterns": ["src/qunit", "src/mocha", "src/libs", "src/nls", "src/legacy"], + "overrides": [{ + "files": ["**/*.ts", "**/*.tsx"], + "extends": [ + "eslint:recommended", + "plugin:vue/recommended", + "plugin:compat/recommended", + "plugin:vuejs-accessibility/recommended", + "plugin:@typescript-eslint/eslint-recommended", + "plugin:@typescript-eslint/recommended" + ], + "parser": "@typescript-eslint/parser", + "parserOptions": { + "ecmaFeatures": { "jsx": true }, + "ecmaVersion": 2018, + "sourceType": "module", + "project": "./tsconfig.json" + }, + "rules": { + "@typescript-eslint/no-explicit-any": 0 + }, + "plugins": ["@typescript-eslint"] + }] } diff --git a/client/.node_version b/client/.node_version index d9289897d30..b460d6f2dea 100644 --- a/client/.node_version +++ b/client/.node_version @@ -1 +1 @@ -16.15.1 +18.12.1 diff --git a/client/babel.config.json b/client/babel.config.json index 9297a9920f7..f3be8a1076f 100644 --- a/client/babel.config.json +++ b/client/babel.config.json @@ -1,5 +1,5 @@ { - "presets": ["@babel/preset-env"], + "presets": ["@babel/preset-env", "@babel/preset-typescript"], "env": { "test": { "plugins": ["@babel/plugin-transform-runtime"] diff --git a/client/docs/composables.md b/client/docs/composables.md new file mode 100644 index 00000000000..2ba75d898fc --- /dev/null +++ b/client/docs/composables.md @@ -0,0 +1,107 @@ +# Composables + +Composables are way of splitting up your code into distinct, reusable chunks. +They can replace providers, mixins and more. Any code you can put into a component, can also be written as a composable. +Using them effectively can make your code more reusable, decoupled, and easier to follow. + +**More about Composables:** + +* [Composables Overview](https://vuejs.org/guide/reusability/composables.html) +* [Composition API](https://vuejs.org/api/composition-api-setup.html) +* [\ +``` + +You can now access the current user with `currentUser.value`. + +## Using Composables in the Options API + +Composables are not limited to the composition api. This is the same example from above, using the options api. + +```vue + +``` + +You can now access the current user with `this.currentUser` from anywhere within the component. + +## Testing Components with Composable Stores + +When writing a test which includes a component that has a composable store (like useCurrentUser), +there are two ways to test it. + +### Mocking the store + +You can provide the store in the mount function as follows: + +```js +const wrapper = shallowMount(TestedComponent, + localVue, + provide: { store }, +}); +``` + +`store` must be a Vuex store. +The `mockModule` helper can help creating a store for the required modules: + +```js +const store = new Vuex.Store({ + modules: { + user: mockModule(userStore), + }, +}); +``` + +### Mocking the composable + +The second option is to mock the composable: + +```js +import { useCurrentUser } from "composables/user"; + +jest.mock("composables/user"); +useCurrentUser.mockReturnValue({ + currentUser: {} +}); +``` + +While simpler in this example, you may need to manually mock more return values and composables than the other method, depending on the composables the component is using. + +## Using Composables for more than Stores + +Composables can be of great use to extract any reactive code from your components. For an example of this, take a look at [userFilterObjectArray](https://github.com/galaxyproject/galaxy/blob/dev/client/src/composables/utils/filter.js). + +Usage: + +```vue + +``` + +It's a simple filtering function, but fully reactive. +Whenever any of the inputs changes, the return value is re-computed, without having to call the function again. diff --git a/client/docs/providers-and-renderers.md b/client/docs/providers-and-renderers.md index 447e6d74b74..480bef218ef 100644 --- a/client/docs/providers-and-renderers.md +++ b/client/docs/providers-and-renderers.md @@ -1,3 +1,7 @@ +**Notice** Consider using [Composables](composables.md) instead of Providers. They offer more functionality and need less boilerplate. + +--- + We are using components in two very distinct ways. The first, "normal", kind of component will probably look familiar to anybody whis is already passingly familiar with Vue. Here the relevant information comes in as properties, any internal variables get defined in "data", changes go out as @@ -27,7 +31,7 @@ restriction that Vue needs a single root element in which to render. @@ -35,7 +39,7 @@ restriction that Vue needs a single root element in which to render. + + diff --git a/client/src/components/Form/Elements/FormSelection.vue b/client/src/components/Form/Elements/FormSelection.vue new file mode 100644 index 00000000000..4ba5e7dd90f --- /dev/null +++ b/client/src/components/Form/Elements/FormSelection.vue @@ -0,0 +1,54 @@ + + + diff --git a/client/src/components/Form/FormDisplay.vue b/client/src/components/Form/FormDisplay.vue index 1c1fcd87bb3..bad270f506e 100644 --- a/client/src/components/Form/FormDisplay.vue +++ b/client/src/components/Form/FormDisplay.vue @@ -57,11 +57,11 @@ export default { }, collapsedEnableIcon: { type: String, - default: "fa fa-caret-square-o-down", + default: "far fa-caret-square-down", }, collapsedDisableIcon: { type: String, - default: "fa fa-caret-square-o-up", + default: "far fa-caret-square-up", }, validationScrollTo: { type: Array, diff --git a/client/src/components/Form/FormElement.test.js b/client/src/components/Form/FormElement.test.js index a8589f74ddd..c361c6b616a 100644 --- a/client/src/components/Form/FormElement.test.js +++ b/client/src/components/Form/FormElement.test.js @@ -1,6 +1,8 @@ import { mount } from "@vue/test-utils"; import { getLocalVue } from "jest/helpers"; import FormElement from "./FormElement"; +import FormHidden from "./Elements/FormHidden"; +import FormInput from "./Elements/FormInput"; const localVue = getLocalVue(); @@ -17,53 +19,80 @@ describe("FormElement", () => { title: "title_text", }, localVue, - stubs: { - FormInput: { template: "
form-input
" }, - FormHidden: { template: "
form-hidden
" }, - }, }); }); it("check props", async () => { const help = wrapper.find(".ui-form-info"); expect(help.text()).toBe("help_text"); + const error = wrapper.find(".ui-form-error-text"); expect(error.text()).toBe("error_text"); + await wrapper.setProps({ error: "" }); const no_error = wrapper.findAll(".ui-form-error"); expect(no_error.length).toBe(0); + const title = wrapper.find(".ui-form-title"); - expect(title.text()).toBe("title_text"); + expect(title.text()).toContain("title_text"); }); it("check collapsibles and other features", async () => { await wrapper.setProps({ disabled: true }); expect(wrapper.findAll(".ui-form-field").length).toEqual(0); + await wrapper.setProps({ disabled: false }); expect(wrapper.findAll(".ui-form-field").length).toEqual(1); - await wrapper.setProps({ default_value: "default_value", collapsible_value: "collapsible_value" }); + + await wrapper.setProps({ + attributes: { default_value: "default_value", collapsible_value: "collapsible_value" }, + }); expect(wrapper.find(".ui-form-title-text").text()).toEqual("title_text"); - expect(wrapper.findAll("span[title='Disable']").length).toEqual(1); + expect(wrapper.findAll("button[title='Disable']").length).toEqual(1); expect(wrapper.emitted().input[0][0]).toEqual("initial_value"); + await wrapper.find(".ui-form-collapsible-icon").trigger("click"); expect(wrapper.emitted().input[1][0]).toEqual("collapsible_value"); expect(wrapper.emitted().input[1][1]).toEqual("input"); + await wrapper.setProps({ collapsedEnableText: "Enable Collapsible", collapsedDisableText: "Disable Collapsible", }); - expect(wrapper.findAll("span[title='Enable Collapsible']").length).toEqual(1); - expect(wrapper.findAll("span[title='Disable Collapsible']").length).toEqual(0); + expect(wrapper.findAll("button[title='Enable Collapsible']").length).toEqual(1); + expect(wrapper.findAll("button[title='Disable Collapsible']").length).toEqual(0); + await wrapper.find(".ui-form-collapsible-icon").trigger("click"); expect(wrapper.emitted().input[2][0]).toEqual("default_value"); - expect(wrapper.findAll("span[title='Disable Collapsible']").length).toEqual(1); - expect(wrapper.findAll("span[title='Enable Collapsible']").length).toEqual(0); + expect(wrapper.findAll("button[title='Disable Collapsible']").length).toEqual(1); + expect(wrapper.findAll("button[title='Enable Collapsible']").length).toEqual(0); }); it("check type matching", async () => { await wrapper.setProps({ type: "text" }); - expect(wrapper.find("div[id='input'").text()).toEqual("form-input"); + expect(wrapper.findComponent(FormInput).exists()).toBe(true); + expect(wrapper.findComponent(FormHidden).exists()).toBe(false); + await wrapper.setProps({ attributes: { titleonly: true } }); - expect(wrapper.find("div[id='input'").text()).toEqual("form-hidden"); + expect(wrapper.findComponent(FormHidden).exists()).toBe(true); + expect(wrapper.findComponent(FormInput).exists()).toBe(false); + }); + + it("marks required values", async () => { + await wrapper.setProps({ type: "text", attributes: { optional: false } }); + expect(wrapper.find(".ui-form-title-star").exists()).toBe(true); + expect(wrapper.find(".ui-form-title-message").exists()).toBe(false); + }); + + it("marks optional values", async () => { + await wrapper.setProps({ type: "text", attributes: { optional: true } }); + expect(wrapper.find(".ui-form-title-star").exists()).toBe(false); + expect(wrapper.find(".ui-form-title-message").text()).toContain("optional"); + }); + + it("warns about empty required values", async () => { + await wrapper.setProps({ type: "text", value: "", attributes: { optional: false } }); + expect(wrapper.find(".ui-form-title-star").exists()).toBe(true); + expect(wrapper.find(".ui-form-title-message").text()).toContain("required"); }); }); diff --git a/client/src/components/Form/FormElement.vue b/client/src/components/Form/FormElement.vue index c1c387f5fea..bfcdf8d831a 100644 --- a/client/src/components/Form/FormElement.vue +++ b/client/src/components/Form/FormElement.vue @@ -1,270 +1,344 @@ + + + + - +.ui-form-element { + margin-top: $margin-v * 0.25; + margin-bottom: $margin-v * 0.25; + overflow: visible; + clear: both; + + .ui-form-title { + word-wrap: break-word; + font-weight: bold; + + .ui-form-title-message { + font-size: $font-size-base * 0.7; + font-weight: 300; + vertical-align: text-top; + color: $text-light; + cursor: default; + } + + .ui-form-title-star { + color: $text-light; + font-weight: 300; + cursor: default; + } + + .warning { + color: $brand-danger; + } + } + + .ui-form-field { + position: relative; + margin-top: $margin-v * 0.25; + } + + &:deep(.ui-form-collapsible-icon), + &:deep(.ui-form-connected-icon) { + border: none; + background: none; + padding: 0; + line-height: 1; + font-size: 1.2em; + + &:hover { + color: $brand-info; + } + + &:focus { + color: $brand-primary; + } + + &:active { + background: none; + } + } +} + diff --git a/client/src/components/Form/FormInputs.vue b/client/src/components/Form/FormInputs.vue index dac24c60abd..139d60b0f3a 100644 --- a/client/src/components/Form/FormInputs.vue +++ b/client/src/components/Form/FormInputs.vue @@ -1,24 +1,28 @@