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

MAINT - Simplify repo and workflow #15

Merged
merged 19 commits into from
Sep 16, 2022
Merged

MAINT - Simplify repo and workflow #15

merged 19 commits into from
Sep 16, 2022

Conversation

trallard
Copy link
Member

@trallard trallard commented Sep 2, 2022

This PR introduces:

  1. Remove abandoned POC
  2. Restructure some bits of the repo, namely: move tools to the top of the repo, align READMEs with content
  3. Tidy up the setup.cfg and pyproject.toml files
  4. Refactor our GH action workflow to use the contents of ./testing/jupyterlab

Closes #3
Closes #9

@trallard
Copy link
Member Author

trallard commented Sep 2, 2022

This is almost done - except for the issue with a missing dependency, it seems (see https://github.com/Quansight-Labs/jupyter-a11y-testing/runs/8156261806?check_suite_focus=true)

@gabalafou, any insights on why this might be happening would be great 🙏🏽 - the dep seems to be in package.json, and yet it is missing?

Affecting:

  1. CI (see actions log)
  2. Local development - (following instructions in readme) throws the same error
code: 'MODULE_NOT_FOUND',
  path: '/home/runner/work/jupyter-a11y-testing/jupyter-a11y-testing/testing/jupyterlab/node_modules/expect-axe-playwright/package.json',
  requestPath: 'expect-axe-playwright'

@gabalafou
Copy link
Contributor

gabalafou commented Sep 6, 2022

any insights on why this might be happening

It was because of a difference in the way that Yarn (1.x) and NPM handle install. My fork added a prepare script to package.json. When you run npm install (in testing/jupyterlab) and it comes across a package (gabalafou/expect-axe-playwright) whose package.json has a prepare script, it downloads both the dependencies and dev dependencies of that included package, which is necessary in order to build the package from source.

So basically, when you run npm install and you do ls node_modules/expect-axe-playwright, you'll see a lib/ directory, but when you do yarn install it doesn't properly execute the prepare script and so there's no lib/ directory, which is why the import from expect-axe-playwright was failing.

I don't know why there's this difference. It's stupid and frustrating, and I think now in 2022, maybe it's no longer beneficial to use Yarn over NPM, so I replaced yarn everywhere with npm.

@trallard
Copy link
Member Author

This should be ready for a final review and merge

@trallard
Copy link
Member Author

@gabalafou gabalafou changed the title MAINT - Simplify repo and worklow MAINT - Simplify repo and workflow Sep 16, 2022
README.md Show resolved Hide resolved
Copy link

@steff456 steff456 left a comment

Choose a reason for hiding this comment

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

I left a couple of minor comments, this PR is looking really good!

@trallard trallard mentioned this pull request Sep 16, 2022
@trallard trallard merged commit 399b911 into main Sep 16, 2022
@trallard trallard deleted the trallard/cleanup branch September 19, 2022 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

[ENH] - Setup documentation and improvements [ENH] - Unify testing structure
3 participants