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

Jump to bottom

Debug and resolve why the E2E tests need --forceExit when run in a Docker container (or on CI) #54

Closed
sebinsua opened this issue Aug 19, 2020 · 3 comments
Closed

Debug and resolve why the E2E tests need --forceExit when run in a Docker container (or on CI) #54

sebinsua opened this issue Aug 19, 2020 · 3 comments
Assignees
@Cono52
Labels
bug Something isn't working high priority

Comments

@sebinsua
Copy link
Contributor

We should debug and resolve why the E2E tests need --forceExit when run in a Docker container (or on CI).

See the following comment for further information:

modular/e2e-tests/core.test.ts

Lines 154 to 168 in 1fd6cb2

// We `cancel` every dev server at the end of each test, but
// for some reason the port is still in use at the beginning
// of the next test.
//
// We avoid this by grabbing the next available port, making
// sure that the next dev server is started using this, and
// passing the correct URL to puppeteer.
//
// TODO: It would be a good idea to try to find out why
// the port is still in use, and why the processes aren't
// being killed. Currently, when the tests are containerised,
// locally or on CI, we have to use `--forceExit` to ensure
// Jest closes.
//
// See: https://github.com/jpmorganchase/modular/pull/45/files#r473007124

@threepointone threepointone added the bug Something isn't working label Sep 6, 2020
@sebinsua sebinsua mentioned this issue Sep 21, 2020
Closed
@sebinsua
Copy link
Contributor Author

sebinsua commented Oct 5, 2020 •
edited
Loading

Note to self: check to see if this is still a problem...

@sebinsua sebinsua self-assigned this Oct 5, 2020
threepointone added a commit that referenced this issue Nov 11, 2020
@threepointone
refactor
fb030c7 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
@threepointone threepointone mentioned this issue Nov 11, 2020
Merged
threepointone added a commit that referenced this issue Nov 11, 2020
@threepointone
refactor
b38f918 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 11, 2020
@threepointone
refactor
aa4f50b 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 11, 2020
@threepointone
refactor
d772ad7 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
fc0a07c 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
40251a1 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
c3d8215 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
ef0a9c3 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
fc4b464 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
59759c7 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
3294fb5 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
aff10c9 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor
4baf0c6 
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
@threepointone
refactor: update dependencies + self host repository. (#132)
1b34e70 
* refactor

This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details - 

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125. 
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were: 
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project. 
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case. 
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR. 

### caveats 

- I need to implement and write tests for the views macro, as mentioned above. 
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so. (EDIT: done, looks fine)
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait. 
- all tests take about 550s on my 2015 macbook pro. That kinda sucks. A lot of the time is literally just fetching dependencies and installing. We should setup caches for our build, and further use cached versions of dependencies when installing. This is less of a problem now that the repo is public, but still. (EDIT: now under 4 minutes, not bad.)
@threepointone
Copy link
Contributor

Marking this as high pri, since it's been marked as skipped in our tests, and we should fix it soon.

@threepointone threepointone mentioned this issue Nov 14, 2020
Closed
20 tasks
@Cono52 Cono52 self-assigned this Nov 23, 2020
@threepointone
Copy link
Contributor

This issue isn't particularly accurate anymore since we've changed the way we run the tests and have a fresh set of problems. Closing this issue in favour of #169.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Cono52 Cono52

Labels
bug Something isn't working high priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@threepointone @sebinsua @Cono52