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

If localStorage is populated via Role, it's empty when running parallel tests #2782

Closed
darina-techery opened this issue Aug 28, 2018 · 21 comments
Assignees
Labels
STATE: Auto-locked An issue has been automatically locked by the Lock bot. TYPE: bug The described behavior is considered as wrong (bug).
Milestone

Comments

@darina-techery
Copy link

darina-techery commented Aug 28, 2018

Are you requesting a feature or reporting a bug?

bug

What is the current behavior?

WHEN tests are executed in parallel (concurrent runs OR multiple browsers)
AND test setup is performed via roles
AND each role populates localStorage to authenticate user
THEN some browser instances have empty localStorage
AND tests fail, because test user is not authorized without respective localStorage entry.

Details:
When user signs in to our application, an app_state property is added to localStorage. It contains a unique connection token. User with this token in localStorage is authorized to perform any operations and has access to the whole application.
localStorage contain the following entry for authorized user:

"app_state": {
    "settings": {},
    "session": {
        "email": "<user_email>",
        "token": "<connection token>"
    }
}

If the tests are executed subsequently, it works as expected. But when I launch tests in multiple browsers or in parallel, a number of tests in a set always fail due to empty localStorage (app_state property is absent). Current user is considered not signed in, and only login page is accessible to him.

Only one browser instance receives properly populated localStorage (probably the one where the role was initialized first?). The other instances have empty localStorages without app_state and tokens.

What is the expected behavior?

Each browser instance maintains its own localStorage, and if some role has already been initialized for one browser instance, it should be restored properly in the others.

My average scenario looks like this:

(in 'before' hook)

  1. Use a role which populates localStorage
  2. (optional) Create/delete test data via REST API
  3. Navigate to the landing page
    (in test itself)
  4. Perform some actions and assertions on UI level.

I expect user to be authorized after step 1 and redirected to the landing page, not login page.

How would you reproduce the current behavior (if this is a bug)?

  1. Pick a site which relies on localStorage state (in our case it's https://app.appspector.com/).
  2. Create a role which adds entries to localStorage.
  3. Create several tests with this role.
  4. Launch tests with concurrent() option or multiple browsers() in runner.
  5. Check localStorage state.

Provide the test code and the tested page URL (if applicable)

Test code

I authenticate my user using REST API ('request' package) and client function writing to localStorage. It looks like this:

const roles = {
    asMainUser: Role(baseUrl, async () => {
        const response = await usersApi.login(email, password);
        const appState = JSON.stringify({
            session: response,
            settings: {}
        });
        await client.localStorageSet('app_state', appState);
    })
};

Most tests have the following structure:

test
    .page(baseUrl)
    .before(async t => {
        await t.useRole(roles.asMainUser);
        await t.navigateTo(baseUrl);
    })
    ('As a user without projects, add new project from Welcome page', async t => {
        await t
            .click(welcomePage.newProject)
            .expect(addAppForm.form.visible).ok()
            .expect(addAppForm.saveApp.hasAttribute('disabled')).ok();
    });

The test above will fail trying to interact with welcome page, as login page will be found instead.

Specify your

  • operating system: MacOS Sierra 10.12.6
  • testcafe version: 0.21.1
  • node.js version: v9.8.0
@AlexKamaev
Copy link
Contributor

AlexKamaev commented Aug 30, 2018

Hi, @darina-techery
I've reproduced the issue and I need time to investigate it.
For now I recommend you use the preserveUrl option in the role constructor and remove page(baseUrl) because every test will start with preserved url.
 
Please see the passing test code below:

import { Role, Selector, ClientFunction } from 'testcafe';
import rp from 'request-promise';

const saveLocalStorage = ClientFunction (storage => {
    window.localStorage.setItem('app_state', storage);
});

const options = {
    method: 'POST',
    uri:    'https://app.appspector.com/api/auth/login',
    body:   {
        email:    '**********',
        password: '**********'
    },
    json: true
};

const userRole = Role('https://app.appspector.com/', async t => {
    const body    = await rp(options);
    const session = JSON.stringify( { session: body, settings: {} });

    await saveLocalStorage(session);
    await t.navigateTo('https://app.appspector.com/welcome');

}, { preserveUrl: true });

fixture `Should restore local storage correctly`
    .beforeEach(async t => {
        await t.useRole(userRole);
    });

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

@darina-techery
Copy link
Author

Hi @AlexKamaev ,

I've already tried that with preserveUrl option, and I can see now that the reason may be not in concurrent execution, but in some concurrency issue, because localStorage can appear empty even in a sequential run. I commented on this in another thread and provided a code sample with preserveUrl there: #2475 (comment)

@AlexKamaev
Copy link
Contributor

@darina-techery
Let's continue in this thread, because the workaround suggested in the issue #2475 is specific for that project.
I'm going to research this issue, but could you please clarify if you tried to run my test? It based on your description and it works stably for me.

@darina-techery
Copy link
Author

Yes, I tried your test, and it runs ok. I'm trying to find out what makes the difference. My first option is moving t.useRole to test.before hook, because most tests contain specific actions in these hooks (mostly CRUD operations via API), and they override the fixture.beforeEach. I'm testing that configuration now.

@AlexKamaev
Copy link
Contributor

In my test, I do not use page because it's not required with the roles mechanism and the preserveUrl option. Besides, I do navigation in the end of role initializer, because I need to navigate to the url, which will be saved in the inner preserveUrl state.

@darina-techery
Copy link
Author

darina-techery commented Aug 31, 2018

Ok, I think I traced the source of this problem via debug output.
My authentication function looks like this (note the debug prints):

const authenticateInLocalStorage = async function(authResponse) {
  const appState = JSON.stringify({
    session: authResponse,
    settings: {}
  });
  console.log("Set app_state: "+appState);
  await client.localStorageSet("app_state", appState);

  console.log("Wait until app_state is found in localStorage");
  await t.expect(client.localStorageGet("app_state")).ok();

  console.log("app_state is found: "+await client.localStorageGet("app_state"));
};

In some cases it works. Example of valid output:

91.53: Set app_state: {"session":{"email":"*****","name":"*****","accountId":*****,"token":"*****","agreementAccepted":true,"id":14062},"settings":{}}
92.92: Wait until app_state is found in localStorage
93.00: app_state is found: {"session":{"email":"***","name":"***","accountId":***,"token":"***","agreementAccepted":true,"id":***},"settings":{}}

But in some cases it results in:

76.03: Set app_state: {"session":{"email":"[email protected]","name":"Yolanda Tillman","accountId":***,"token":"***","agreementAccepted":false,"id":***},"settings":{}}
76.75: Wait until app_state is found in localStorage
77.17: app_state is found: {"settings":{}}

This line app_state is found: {"settings":{}} is clearly indicating that the test is trying to access some wrong localStorage instance, because after JSON.stringify app_state becomes a string, not an object, and a string cannot for some reason lose its substring.
It doesn't happen in all cases though.

For the record, my localStorageSet is localStorageSet: ClientFunction((key, val) => localStorage.setItem(key, val)),. I will try using window.localStorage instead and see what happens.

@AndreyBelym AndreyBelym added the TYPE: bug The described behavior is considered as wrong (bug). label Aug 31, 2018
@AndreyBelym AndreyBelym added this to the Sprint #17 milestone Aug 31, 2018
@AlexKamaev AlexKamaev self-assigned this Sep 4, 2018
@AlexKamaev
Copy link
Contributor

@darina-techery I'm working on the issue and today your site banned me. Could you please help me restore access to the site to continue my research?

@darina-techery
Copy link
Author

@AlexKamaev our security guys have recently blocked a bunch of IPs. They considered multiple sessions spawning as a kind of DDoS attack. I'm terribly sorry for this misunderstanding.
Can you please reach me darina (dot) sunstream (at) gmail.com and tell me your IP? I'll get you unbanned.

@AlexKamaev
Copy link
Contributor

@darina-techery thanks, I've mailed it

@sergeyzenchenko
Copy link

@AlexKamaev I've whitelisted your IP. Sorry for inconvenience :)

@AlexKamaev
Copy link
Contributor

@sergeyzenchenko thanks

@darina-techery
Copy link
Author

I'm sharing a couple of workarounds for those who might encounter the problem before it's fixed:

  1. Use a Role workaround from Role doesn't work when page navigation doesn't trigger page reloading #2195 (comment) (set {preserveUrl: true}, do not use pageUrl in tests/fixtures, do not use t.navigateTo in a role itself).

  2. If your application uses localStorage to authenticate user, and you populate it explicitly, you can try this workaround:

const authenticateInLocalStorage = async function(localStorageEntry) {
  console.debug("Expecting state in localStorage:", localStorageEntry);

  let retryCount = 0;
  const maxRetries = 10;
  const tokenToCheck = localStorageEntry.someToken;
  let failed = false;
  const authFailed = async (localStorageState): Promise<boolean> => {
    return (
        //check if localStorage state matches expected one
        !localStorageState || localStorageState.someToken !== tokenToCheck
    );
  };

  do {
    retryCount++;
    await localStorageSet("state", localStorageEntry);
    const appStateInLS = await localStorageGet("state");
    failed = await authFailed(appStateInLS);
    console.debug(
      (failed ? "Fail" : "Pass") +
        ": state in localStorage is " +
        appStateInLS
    );
  } while (retryCount < maxRetries && failed);

  if (failed) {
    throw new Error(
      "Failed to set state in localStorage: expected " +
        localStorageEntry +
        ", but found " +
        (await localStorageGet("state"))
    );
  }
};

@AlexKamaev
Copy link
Contributor

@darina-techery
I've researched your scenario and found that the cause of the issue can be in caching. The roles mechanism without the preserveUrl option makes two requests to the login page. Both of them should be real requests to the server. However, in response to the first request, the server sends caching headers, so a browser caches the login page. Thus, the second request is not sent to the server, because the login page is in the browser cache.
Could you please check my assumption? I've tried to run Chrome with the flag --disk-cache-size=1 and it solves the problem while running without a flag does not work. Just type in CMD testcafe "chrome --disk-cache-size=1" your_test_file.js.

Here is my tests:

import { Role, Selector, ClientFunction, RequestMock, RequestHook } from 'testcafe';
import rp from 'request-promise';

const saveLocalStorage = ClientFunction (storage => {
    window.localStorage.setItem('app_state', storage);
});

const options = {
    method: 'POST',
    uri:    'https://app.appspector.com/api/auth/login',
    body:   {
        email:    '[email protected]',
        password: '1234567890'
    },
    json: true
};

const mock = RequestMock()
    .onRequestTo(/.*logrocket.*/)
    .respond((req, res) => {
        res.headers['x-calculated-header'] = 'calculated-value';
        res.statusCode = '200';
        res.setBody('body');
    })
    .onRequestTo(/.*google-analytics.*/)
    .respond((req, res) => {
        res.headers['x-calculated-header'] = 'calculated-value';
        res.statusCode = '200';
        res.setBody('body');
    })
    .onRequestTo(/.*mixpanel\.com.*/)
    .respond((req, res) => {
        res.headers['x-calculated-header'] = 'calculated-value';
        res.statusCode = '200';
        res.setBody('body');
    })
    .onRequestTo(/.*countly.*/)
    .respond((req, res) => {
        res.headers['x-calculated-header'] = 'calculated-value';
        res.statusCode = '200';
        res.setBody('body');
    });

const userRole = Role('https://app.appspector.com/login', async t => {
    const body    = await rp(options);
    const session = JSON.stringify( { session: body, settings: {} });

    await saveLocalStorage(session);
    await t.wait(5000);

}, { preserveUrl: false });

fixture `Should restore local storage correctly`
    .page('https://app.appspector.com/welcome')
    .requestHooks(mock);

test('Should be logged in', async t => {
    await t.useRole(userRole);
    await t.wait(4000)
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

test('Should be logged in', async t => {
    await t.useRole(userRole);
    await t.wait(4000)
    await t.click(Selector('div').withText('Welcome to AppSpector'));
});

@darina-techery
Copy link
Author

I tried my tests with this option, but the result was the same.
I captured the aforementioned occasional failure once more: app_state is set properly, but when I fetch it without navigating anywhere, it appears empty:

10.18 [DEBUG] Received auth response, expecting state {"session":{"email":"***","name":"autobot","accountId":14174,"token":"992idhr86kr9j0gs22gve5og4b","agreementAccepted":true,"id":14062},"settings":{}}
10.22 [DEBUG] Pass: app_state in localStorage is {"session":{"email":"***","name":"autobot","accountId":14174,"token":"992idhr86kr9j0gs22gve5og4b","agreementAccepted":true,"id":14062},"settings":{}}
12.73 [DEBUG] Requested token from localStorage. Current app_state is {"settings":{}}

The next failure, which is a constant one, occurred when t.useRole was not the first command in the test. I obtained the token via REST API requests and used it to send a few requests to create test data (in my case, create a test organization and send an invite to another user which should accept it).

const acceptEmail = generators.email();
const acceptRole = Role(
  "https://app.appspector.com/welcome",
  async () => {
    // register via API and set app_state in localStorage;
    await registerAs(acceptEmail);
  },
  { preserveUrl: true }
);

//I used this workaround, because in some cases it worked, but now it failed:
export const useRoleAndReload = async (t, role: Role) => {
  await t.useRole(role).wait(1000);
  await client.reload();
};

test
  ("Accept invite to organization",
  async t => {
//create user session via API
    const mainUserSession = await usersApi.login(
      mainUser.email,
      mainUser.password
    );
//save token to t.ctx.token
    await requests.setToken(mainUserSession.token);
//create organization as main user (token is automatically taken from t.ctx.token)
    const organization = await orgsApi.createOrganization();
    await invitesApi.inviteMember({
      orgId: organization.id,
      email: acceptEmail,
      role: "member"
    });

//now we try to register a user with an email we used to send invite to
    await useRoleAndReload(t, acceptRole);
    await t
      .expect(welcomePage.viewInvites.textContent)
      .eql("View 1 Invite", "View Invites button")
  }
);

This test also fails, as I see login page on the screen.
My launch command is yarn tsc && testcafe "chrome --disk-cache-size=1" build/tests/main/invites_tests.js -s results/screenshots -S --skip-js-errors

@darina-techery
Copy link
Author

@AlexKamaev Oh, I see I still used preserveUrl: true. I will try false and let you know what happens.

@darina-techery
Copy link
Author

@AlexKamaev I changed the role approach (now tests contain .page() setting, roles do not include navigateTo, preserveUrl is false) and used --disk-cache-size=1 launch argument.
The tests continue to fail randomly (probably in 10% cases). I added an afterEach statement to each test and log the current app_state value in it. When a test fails, localStorage is in a non-authorized state ({"settings":{}}). When a test passes, localStorage contains valid session entry.

I can also experiment with increasing the t.wait() timeout, it is set to 500 ms now. I will check if increasing it to 4000 ms does the job, but this behavior would be highly undesirable in terms of tests performance.

@AlexKamaev
Copy link
Contributor

Since the issue disappeared on my side after disabling the cache, the reason of the issue can be not only in the cache. Could you please run your tests with my packed testcafe versions? One version just logs the requests, the second version has some modifications. Could you share these logs with me?
Please follow the next steps:

  1. unpack the attachement
  2. modify package.json
    use "testcafe": "./reset_hash/testcafe-0.22.0.tgz" for version with modifications
    "testcafe": "./just_log/testcafe-0.22.0.tgz" just for logging
  3. Run npm i
  4. Replace node_modules\testcafe-hammerhead\lib with hammerhead_lib.rar\lib
  5. Run my tests and yours
     
    I hope that logs will help me to understand the reason of the issue.
    Could you also share your test files to me via email?

darina-techery.zip

@AlexKamaev
Copy link
Contributor

@darina-techery
I've discovered an interesting point. I removed request mocks just for a couple of test runs and removed the --skip-js-errors option. With that configuration, I was able to reproduce the issue even with the disabled cache. So, could you please allow me to disable mocks so I research the issue further?

@darina-techery
Copy link
Author

darina-techery commented Sep 12, 2018

@AlexKamaev
Unfortunately, I cannot remove --skip-js-errors, as it is required to suppress certain errors produced by React components rendering. It would be nice to have a possibility to suppress not all errors, but selected ones.

I have built a sample project which guarantees reproducibility, and it doesn't seem to be connected to requestMocks. The failures look completely random to me, and sometimes changing something in the test might make it look like as if the problem was fixed, but then the localStorage issue emerges again without any further modifications :(

But so far I would prefer to leave the sample project private, unless we are unable to resolve the issue without the code being exposed (NDA and stuff). Could you please check your email? I messaged you with some details regarding this investigation. Thanks!

@darina-techery
Copy link
Author

The issue is solved now. As @AlexKamaev found, the problem was caused by redux-localstorage app module. It overwrote localStorage state, which led to invalid app state and occasionally crashed random tests. Alex also offered a workaround to fix this:

  1. Add a client function to backup local storage content:
export const client = {
  backupStorage: ClientFunction(appState => {
    window.localStorage.setItem("app_state", appState);

    return window['%hammerhead%'].sandbox.storageSandbox.backup();
  }),
  1. If you use a custom function to set up localStorage via role mechanism, pass your role as one more argument and create a hook to initialize role with proper localStorage state retrieved from backup:
const setUpLocalStorageForRole = async (args, role) => {  
  //perform setup actions, populate localStorage
  const backup = await client.backupStorage(appState)

  role.once('initialized', async () => {
    role.stateSnapshot.storages = backup;
  });

Thanks so much for you help! I think we're settled here.

@lock
Copy link

lock bot commented Mar 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or feature requests. For TestCafe API, usage and configuration inquiries, we recommend asking them on StackOverflow.

@lock lock bot added the STATE: Auto-locked An issue has been automatically locked by the Lock bot. label Mar 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
STATE: Auto-locked An issue has been automatically locked by the Lock bot. TYPE: bug The described behavior is considered as wrong (bug).
Projects
None yet
Development

No branches or pull requests

4 participants