Bug?: `cy.task('jambox.config'` with stub config seems to break forward config maybe?

[hamlim]

Details

[!NOTE] Might take me a bit to create a reproduction of this outside of the app

While upgrading to 0.1.1, the following cypress test case seemed to fail, moving the jambox.config cypress task below the first visit seemed to resolve the failure however.

Initial Cypress test case:

it('asserts the content on the following page is what we expect', () => {
  const DEPT = 'furniture';
  // Stub out the /furniture/ page so that we don't perform a full page load once the link is clicked
  cy.task('jambox.config', {
    stub: {
      '**/furniture': {
        preferNetwork: true,
        status: 200,
        body: {text: 'Furniture'},
      },
    },
  });
  cy.visit('/');
  cy.get('[data-test-id="collection"]', {timeout: 10000})
    .find('[data-test-id="heading"]')
    .eq(0)
    .should('contain.text', 'Shop');

  cy.get('[data-test-id="collection"]', {timeout: 10000})
    .find('[data-test-id="furniture-link"]')
    .contains(DEPT, {matchCase: false})
    .click();
  cy.url().should('contain', DEPT, {matchCase: false});

  cy.task('jambox.reset');
});

It failed with the following error:

cy.visit() failed trying to load:

https://example.com/

The response we received from your web server was:

  > 503: Service Unavailable

This was considered a failure because the status code was not 2xx.

(swapped out real host for example.com in the above error)

Changing the test to instead move the cy.task below the first visit made it pass successfully:

it('asserts the content on the following page is what we expect', () => {
  const DEPT = 'furniture';
  cy.visit('/');
  // Stub out the /furniture/ page so that we don't perform a full page load once the link is clicked
  cy.task('jambox.config', {
    stub: {
      '**/furniture': {
        preferNetwork: true,
        status: 200,
        body: {text: 'Furniture'},
      },
    },
  });

  cy.get('[data-test-id="collection"]', {timeout: 10000})
    .find('[data-test-id="heading"]')
    .eq(0)
    .should('contain.text', 'Shop');

  cy.get('[data-test-id="collection"]', {timeout: 10000})
    .find('[data-test-id="furniture-link"]')
    .contains(DEPT, {matchCase: false})
    .click();
  cy.url().should('contain', DEPT, {matchCase: false});

  cy.task('jambox.reset');
});

TL;DR:

Wondering if the cypress task changed the behavior somehow between 0.0.30 and 0.1.1. I think we tried 0.1.0 out and ran maybe into this failure and put that work off for the time being until the fix landed for #42 in 0.1.1.

[hamlim]

Attempted to create a reproduction of the issue here, however the jambox.config cy.task doesn't seem to be taking correctly - maybe I misconfigured something...

https://github.com/hamlim/jambox-cy-task-config-issue

Of note - it'd be neat to have cypress configured in the recipe app as an example to look at also!

[ballercat]

Okay, I'll take a look at repro

Of note - it'd be neat to have cypress configured in the recipe app as an example to look at also!

Sure! I can check in the example you provide once I figure out what the issue is.

[ballercat]

@hamlim

So both [broken] and [working] examples pass for me in the example repo. The issue you had was that body was set to a string, but it should have been an object.

diff --git a/cypress/e2e/app.cy.ts b/cypress/e2e/app.cy.ts
index 42f5b7b..839e573 100644
--- a/cypress/e2e/app.cy.ts
+++ b/cypress/e2e/app.cy.ts
@@ -6,7 +6,7 @@ describe('Navigation', () => {
         '**/foo': {
           preferNetwork: false,
           status: 200,
-          body: `You're visiting foo!`
+          body: { text: `You're visiting foo!` }
         }
       }
     })
@@ -22,7 +22,7 @@ describe('Navigation', () => {
         '**/foo': {
           preferNetwork: false,
           status: 200,
-          body: `You're visiting foo!`
+          body: { text: `You're visiting foo!` }
         }
       }
     })

The whole "body should be an object" thing is not great though I can patch that to be a bit more easy to work with.

[hamlim]

Oddly, in the real app I was seeing the page that the test navigates to contain the literal: {body: "You're visiting foo!" } - so I dropped the {} part, but maybe that's important and something else is going on in our app...

[hamlim]

Alright, now I'm not sure why/where its going wrong in our real app, feel free to close this for now!

[ballercat]

@hamlim

I think what's going on is that there is a race condition between setting the config and jambox resetting. It's not really a problem with your App, but rather how the /api/config post call is handled.

Internally, the main jambox class is listening to config events one of which is an update. When it notices an update to the config it performs a reset. If I had to bet, the amount of caches loaded is large and is causing the reset to take a while in comparison to a bare-bones app like the one in the repro.

It's also not intentional for the jambox.config cypress task to work like that. It should yield when jambox is fully reset but it's not really doing that currently. It's is kind of weird that it's working after the cy.visit though.

[ballercat]

So I can patch this, but I'll have a problem with actually testing if it's working due to me not having access to an app with a large cache locally. If you're interested I can push up a branch and you can test it yourself?

[hamlim]

Yea that'd be great - is there a way we can emulate a large cache with my test repo also? Maybe an API endpoint in the app plus a for loop in an effect to fetch from the cache possibly?

[ballercat]

Hmm... I mean I can put in a slowdown internally into jambox locally to emulate the issue but the only other way to do is to have a large cache zip file.

[ballercat]

@hamlim

Okay #45 should fix this. I tested it locally on the example repo. I took a .json mock file from the recipe folder and cloned it 1K times then zipped it up to replicate a large zip file.

for i in {1..999}; do cp mock.json "mock$i.json"; done
dir2tape.js $(pwd)
mv out.tape.zip default.tape.zip

I have a script file in the repo to https://github.com/ballercat/jambox/blob/76d01ada0f72396800cf6bd090c5275a1ebeebb3/scripts/dir2tape.mjs to zip normal json files into a zip archive.

I can publish #45 since the change is backwards compatible. But it would be cool if you could test it first 👍

[hamlim]

Cool - fighting a separate fire at the moment, I'll test this out after I get the fire put out 😂