Native UT for cordova plugin (android) #555

The test classes are in the node subdirectory of src/android/test directory. There are also few mocks there. There is also the modification of the build script and androidBeforeCompile.js allowing to copy UT files in case when the "./build.sh UT" is issued. The test runner "ThaliTestRunner" is registered to be executed in the JXcoreExtension. If the class RegisterExecuteUT exists, which means that all test classes are copied, the ExecuteNativeTests method is registered in the jxcore and executed.  At the end there is also modification of the app.js allowing to execute UT before the e2e test. The modification of the appveyor.yml was, because there are some issues when using the latest cordova version.

This change is

PR is added to the queue for testing as 1. task. (35c04ce)

Hi @jareksl, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
The agreement was validated by Microsoft and real humans are currently evaluating your PR.
TTYL, MSBOT;

Test (Fail) 77622416 build is completed (35c04ce)
See https://github.com/ThaliTester/TestResults/tree/7762241635c04ce_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

src/android/test/io/jxcore/node/jxcore.java, line 1 [r1] (raw file):

// License information is available from LICENSE file

This file should be removed, the whole modification is now done in the JXcoreExtension

Comments from Reviewable

scripts/androidBeforeCompile.js, line 54 [r1] (raw file):

};

var replaceJXCore = function (appRoot) {

This function should be removed, as the jxcore is not replaced anylonger

Comments from Reviewable

Reviewed 14 of 36 files at r1.
Review status: 14 of 36 files reviewed at latest revision, 12 unresolved discussions, some commit checks failed.

scripts/androidBeforeCompile.js, line 54 [r1] (raw file):
Previously, jareksl (Jarosław Słota) wrote…
> This function should be removed, as the jxcore is not replaced anylonger

I don't remember anymore what it was that we were fixing but what changed that we don't need to fix it anymore?

scripts/androidBeforeCompile.js, line 77 [r1] (raw file):

  var targetFile = path.join(appRoot, 'platforms/android/src/io/jxcore/node');
  try {
    fs.copySync(sourceFile, targetFile);

There is the same code structure here three times, can we replace this and the next two functions with a call to a single function that takes the source, target and some kind of error message as args and then does the copySync and handles the error by exiting?

scripts/androidBeforeCompile.js, line 146 [r1] (raw file):

var copyTestFiles = function (appRoot) {
  try {
    var st = fs.lstatSync('platforms/android/unittests');

Please use a more descriptive name than 'st'

src/android/java/io/jxcore/node/IncomingSocketThread.java, line 42 [r1] (raw file):

     * @throws IOException Thrown, if the constructor of the base class, SocketThreadBase, fails.
     */
    public IncomingSocketThread(BluetoothSocket bluetoothSocket, Listener listener,

Is this constructor just for testing purposes? If so we should mark it as such in the docs.

src/android/java/io/jxcore/node/JXcoreExtension.java, line 81 [r1] (raw file):

     */

    private static void RegisterUTExecuteMethod() {

I'm really not happy with having this code in our mainline JXcoreExtension. If I'm reading this right then this version of JXcoreExtension would be in our production code and we are just depending on the class detector failing to make sure it doesn't run. But that just isn't good coding practice. We don't mix our test code with our production code.
In fact, why is this here at all? I thought the UT tests are pure native Android. Why do we need JXcoreExtension? Please explain on our next stand up. If we do need to call out to this then I would rather we monkey patched this file as part of the test install environment.

src/android/java/io/jxcore/node/JXcoreExtension.java, line 99 [r1] (raw file):

    private static Class<?> RetrieveRegisterUTClass() {
        try {
            return Class.forName("com.test.thalitest.RegisterExecuteUT");

Shouldn't we put the names in our own actual DNS registered namespace? Something like org.thaliproject.java.thalitest.RegisterExecuteUT?

src/android/java/io/jxcore/node/ListenerOrIncomingConnection.java, line 59 [r1] (raw file):

    public int getClientPortNumber() {
        return mClientPortNumber;

If we lived with a bug this bad do we really need this method?

src/android/java/io/jxcore/node/OutgoingSocketThread.java, line 39 [r1] (raw file):

     * @param listener        The listener.
     * @param inputStream     The InputStream.
     * @param outputStream    The OutputStream.

I'm guessing this is another test specific method? If so please mark it as such.

src/android/java/io/jxcore/node/SocketThreadBase.java, line 63 [r1] (raw file):

     * @param outputStream    The OutputStream.
     */
    public SocketThreadBase(BluetoothSocket bluetoothSocket, Listener listener,

I promise, this will be the last time I make this specific comment. But anytime we put in a constructor or method just for testing purposes we should prominently mark it as such so people trying to read the code don't get confused. We will also eventually need to exclude these methods when we do code coverage tests.

src/android/test/com/test/thalitest/MainActivity.java, line 2 [r1] (raw file):

/*
       Licensed to the Apache Software Foundation (ASF) under one

Um... this isn't our copyright notice. We use MIT. Why is this here?

src/android/test/com/test/thalitest/ThaliTestSuite.java, line 19 [r1] (raw file):

@RunWith(Suite.class)
@Suite.SuiteClasses({

Is there no way to generate this list dynamically? The reason I ask is that it wouldn't be the 1st or 10th time a framework that required explicitly listing tests resulted in a situation where people adding new tests didn't realize they had to register them and they never get run and nobody every notices. :(

Comments from Reviewable

Reviewed 22 of 36 files at r1.
Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 1 [r1] (raw file):

package io.jxcore.node;

Have y'all tried to run any kind of code coverage statistics so we can see how much of the code we are actually covering?

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 74 [r1] (raw file):

    @After
    public void tearDown() throws Exception {
        mConnectionHelper.killAllConnections();

Please put this code and its equivalent version in tearDownAfterClass into a single function and call that.
Or, alternatively, why are we repeating the code at all? If we are going to shut everything down after every test then why do we need to do it at the end of the entire class's test run?

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 659 [r1] (raw file):

    @Test
    public void testOnPeerUpdated() throws Exception {
        assertThat(true, is(true));

Yes, it's true that true is true but I"m guessing something is actually supposed to be here?

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 672 [r1] (raw file):

        mIncomingSocketThreadMock.setPeerProperties(mPeerProperties);
        mConnectionModel.addConnectionThread(mIncomingSocketThreadMock);
        mConnectionHelper.onPeerLost(mPeerProperties);

Is this actually checking that onPeerLost is called?

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 684 [r1] (raw file):

    public void testOnProvideBluetoothMacAddressRequest() throws Exception {
        thrown.expect(UnsupportedOperationException.class);
        mConnectionHelper.onProvideBluetoothMacAddressRequest("1111");

Shouldn't we be checking that this one and the next two do call the handlers when they are supposed to be?

src/android/test/io/jxcore/node/IncomingSocketThreadTest.java, line 1 [r1] (raw file):

package io.jxcore.node;

The most interesting part of this class is the run method and it doesn't seem to be tested

src/android/test/io/jxcore/node/jxcore.java, line 1 [r1] (raw file):
Previously, jareksl (Jarosław Słota) wrote…
> This file should be removed, the whole modification is now done in the JXcoreExtension

Is this still the case? I'm ignoring this file for the moment until I get clarification. Thanks!

src/android/test/io/jxcore/node/OutgoingSocketThreadTest.java, line 1 [r1] (raw file):

package io.jxcore.node;

Run is where all the fun is but it isn't being tested!

src/android/test/io/jxcore/node/StreamCopyingThreadTest.java, line 71 [r1] (raw file):

    @Test
    public void testRun() throws Exception {

Given how earth shatteringly important this code is to everything we do wouldn't it make sense to test that we can handle more than just copying a single string? I would want to test several different passes of data just to make sure the engine works right.

src/android/test/io/jxcore/node/StreamCopyingThreadTest.java, line 103 [r1] (raw file):

        @Override
        public void onStreamCopyError(StreamCopyingThread who, String errorMessage) {

Shouldn't we get unhappy if this is called? And actually shouldn't we test how we behave in that case?

test/www/jxcore/UnitTest_app.js, line 33 [r1] (raw file):

      });
    })
      .then(function (result) {

NB: Please indent this code one, out style is to put .then on the new line at the same level as the previous promise code.

test/www/jxcore/UnitTest_app.js, line 45 [r1] (raw file):

        }

        Promise.all(promiseList)

I"m confused why the previous code was shoved between starting the radios if they aren't on and checking if the radios actually go started. Those are two pieces of heavily related code. I suspect the previous promise which runs UT should be first and in fact should probably run before the getNetworkStatus call?

thali/install/setUpTests.sh, line 66 [r1] (raw file):

cp -v $1 app.js

# In case of UT crete a file

NB: /crete/create

thali/install/setUpTests.sh, line 67 [r1] (raw file):

# In case of UT crete a file
if [ $2 == "UT" ]; then

Rather than having two if statements how about just using OR?

thali/install/setUpTests.sh, line 68 [r1] (raw file):

# In case of UT crete a file
if [ $2 == "UT" ]; then
  echo "UT files will be copied to the platform directory"

Come on, you can set a shell variable with that string instead of repeating it three times

Comments from Reviewable

Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

scripts/androidBeforeCompile.js, line 54 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> I don't remember anymore what it was that we were fixing but what changed that we don't need to fix it anymore?

We were executing out UT in jxcore_ready method of jxcore.java. Now we are monkey patching JXcoreExtension, so we do not need it anymore.

scripts/androidBeforeCompile.js, line 77 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> There is the same code structure here three times, can we replace this and the next two functions with a call to a single function that takes the source, target and some kind of error message as args and then does the copySync and handles the error by exiting?

Agree, will be fixed.

Comments from Reviewable

Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

src/android/java/io/jxcore/node/JXcoreExtension.java, line 81 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> I'm really not happy with having this code in our mainline JXcoreExtension. If I'm reading this right then this version of JXcoreExtension would be in our production code and we are just depending on the class detector failing to make sure it doesn't run. But that just isn't good coding practice. We don't mix our test code with our production code.
>
> In fact, why is this here at all? I thought the UT tests are pure native Android. Why do we need JXcoreExtension? Please explain on our next stand up. If we do need to call out to this then I would rather we monkey patched this file as part of the test install environment.

Agree, scripts are already written.

src/android/java/io/jxcore/node/JXcoreExtension.java, line 99 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Shouldn't we put the names in our own actual DNS registered namespace? Something like org.thaliproject.java.thalitest.RegisterExecuteUT?

The reason why we put this in com.test.thalitest is that Cordova by default creates this package while creating project, which we named thalitest and puts there MainActivity.java, the file where all starts from. So we just adjusted to it to not make a bigger mess.

Comments from Reviewable

Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

src/android/test/com/test/thalitest/MainActivity.java, line 2 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Um... this isn't our copyright notice. We use MIT. Why is this here?

Will be replaced by MIT license.

src/android/test/com/test/thalitest/ThaliTestSuite.java, line 19 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Is there no way to generate this list dynamically? The reason I ask is that it wouldn't be the 1st or 10th time a framework that required explicitly listing tests resulted in a situation where people adding new tests didn't realize they had to register them and they never get run and nobody every notices. :(

Scripts are already written.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 74 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Please put this code and its equivalent version in tearDownAfterClass into a single function and call that.
>
> Or, alternatively, why are we repeating the code at all? If we are going to shut everything down after every test then why do we need to do it at the end of the entire class's test run?

tearDownAfterClass will be removed.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 659 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Yes, it's true that true is true but I"m guessing something is actually supposed to be here?

This method returns nothing, all it does is some logging about peer change, so we won't test it anymore.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 684 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Shouldn't we be checking that this one and the next two do call the handlers when they are supposed to be?

This methods only throw exception when they are called, there is no handler to call. They override methods from DiscoveryManager.

src/android/test/io/jxcore/node/IncomingSocketThreadTest.java, line 1 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> The most interesting part of this class is the run method and it doesn't seem to be tested

There is some problem with testing run method. After starting this thread, exception is thrown so we are unable to test it.
Exception: Failed to create the local streams: failed to connect to localhost/<IP_ADDRESS> (port 0): connect failed: ECONNREFUSED (Connection refused)

src/android/test/io/jxcore/node/OutgoingSocketThreadTest.java, line 1 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Run is where all the fun is but it isn't being tested!

Test is already written. But, at some point, this socket/thread is waiting for some incoming connection, so either we will simulate some incoming connection or just test if everything is okay before blocking call.

test/www/jxcore/UnitTest_app.js, line 45 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> I"m confused why the previous code was shoved between starting the radios if they aren't on and checking if the radios actually go started. Those are two pieces of heavily related code. I suspect the previous promise which runs UT should be first and in fact should probably run before the getNetworkStatus call?

Yes, UT should execute before checking radios and before e2e tests.

Comments from Reviewable

Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

scripts/androidBeforeCompile.js, line 54 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> We were executing out UT in jxcore_ready method of jxcore.java. Now we are monkey patching JXcoreExtension, so we do not need it anymore.

our*

Comments from Reviewable

Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

src/android/java/io/jxcore/node/JXcoreExtension.java, line 81 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> Agree, scripts are already written.

And the reason why we are firing UT from JXcoreExtension is that we need to wait for jxcore to load properly.

Comments from Reviewable

Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 1 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Have y'all tried to run any kind of code coverage statistics so we can see how much of the code we are actually covering?

Code coverage could be a problem because we are actually using device resources in our tests so they are not exactly UT but rather Instrumentation tests. I don't think there are tools that provide code coverage on non-rooted phones. Also, our tests are not in different package than classes we are testing, because this classes are avalaible only in their package, so this could be also a problem for coverage tools. I will look into that.

Comments from Reviewable

src/android/java/io/jxcore/node/ListenerOrIncomingConnection.java, line 59 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> If we lived with a bug this bad do we really need this method?

None of the methods: getClientPortNumber() , setClientPortNumber(),  getServerPortNumber() , setServerPortNumber() is used, that's the reason why we didn't spot this bug before. Should we remove them if we don't use them?

Comments from Reviewable

src/android/test/com/test/thalitest/MainActivity.java, line 2 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> Will be replaced by MIT license.

This file is generated by Cordova. It is leftover from the previous implementation, now it is not needed. Will be removed from the repository.

Comments from Reviewable

src/android/test/io/jxcore/node/jxcore.java, line 1 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Is this still the case? I'm ignoring this file for the moment until I get clarification. Thanks!

Yes, this file is not modified anylonger.

Comments from Reviewable

PR is added to the queue for testing as 2. task. (6f426cd)

PR is added to the queue for testing as 3. task. (6db488b)

Reviewed 14 of 14 files at r2.
Review status: all files reviewed at latest revision, 29 unresolved discussions, some commit checks failed.

scripts/androidBeforeCompile.js, line 59 [r2] (raw file):

  try {
    console.log('Copying ' + message);

NB: Is it really worth logging this? We are already really chatty in our logs and it makes finding errors difficult. if something fails then we get a log in the error handler so maybe we can get rid of this log?

scripts/androidBeforeCompile.js, line 112 [r2] (raw file):

 * @param {Object} appRoot
 */
// jscs:enable jsDoc

Why did you have to disable jsdoc? We should be passing jsdoc validation.

scripts/androidBeforeCompile.js, line 143 [r2] (raw file):

 * @param {Object} appRoot
 */
// jscs:enable jsDoc

Same issue, why are we disabling jsOoc validation? Please apply this comment to the rest of the file. :)

scripts/androidBeforeCompile.js, line 175 [r2] (raw file):

// jscs:disable jsDoc
/**
 * Updates JXcoreExtension with a method is used to register the native UT executor

Please throw in a note that we are doing this because we do not want to edit the original extension file that goes into production with test code so instead we add it dynamically only when we are testing.

src/android/java/io/jxcore/node/JXcoreExtension.java, line 96 [r2] (raw file):

        });

        lifeCycleMonitor.start();

We should probably stick in a comment here informing the reader that the native tests plan on sticking their content dynamically after this line so anyone editing this line needs to go check out updateJXCoreExtensionWithUTMethod in androidBeforeCompile.js

src/android/java/io/jxcore/node/ListenerOrIncomingConnection.java, line 59 [r1] (raw file):
Previously, jareksl (Jarosław Słota) wrote…
> None of the methods: getClientPortNumber() , setClientPortNumber(),  getServerPortNumber() , setServerPortNumber() is used, that's the reason why we didn't spot this bug before. Should we remove them if we don't use them?

If we aren't using them, kill 'em. They are certainly easy to put back in later.

src/android/test/com/test/thalitest/UTMethod, line 10 [r2] (raw file):

    private static void RegisterUTExecuteMethod() {
        Class<?> clazz = RetrieveRegisterUTClass();

Isn't this code actually bad in the sense that if we have included UTMethod then we did so because we expect to run tests and if tests don't run because the class isn't there then we know something horrible has gone wrong and we should have an error.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 659 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> This method returns nothing, all it does is some logging about peer change, so we won't test it anymore.

Actually this is a pretty seriously critical method since it what tells JXcore when we have found a peer. That's a pretty big deal. Shouldn't we test that it's getting called correctly?
If you agree then please open a separate issue to add the test. I don't want to hold up this PR over writing that test.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 684 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> This methods only throw exception when they are called, there is no handler to call. They override methods from DiscoveryManager.

So shouldn't we be testing that they throw? And if so then please add as a separate bug as I don't want to hold up this PR over this issue.

src/android/test/io/jxcore/node/IncomingSocketThreadTest.java, line 1 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> There is some problem with testing run method. After starting this thread, exception is thrown so we are unable to test it.
> Exception: Failed to create the local streams: failed to connect to localhost/<IP_ADDRESS> (port 0): connect failed: ECONNREFUSED (Connection refused)

It is trying to connect in order to move data. The whole point of the class. So we need to give it an endpoint to connect to (it's localhost so we are good there) and make sure we are behaving right. Please add an issue that we should test this but let's not hold up the PR over it.

src/android/test/io/jxcore/node/OutgoingSocketThreadTest.java, line 1 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> Test is already written. But, at some point, this socket/thread is waiting for some incoming connection, so either we will simulate some incoming connection or just test if everything is okay before blocking call.

Right, we have to create something that creates a local socket and sends in data. This all looks like normal sockets so testing it should be easy (ish). As with the other similar comments please just file a bug on this issue, I don't want to hold up the PR for it.

src/android/test/io/jxcore/node/OutgoingSocketThreadTest.java, line 75 [r2] (raw file):

        assertThat("mServerSocket.isBound should return true", mServerSocket.isBound(),
                is(true));
        //TODO Simulate incoming connection

yeah, exactly. I'm o.k. though with us filing this as a separate issue and dealing with it later.

test/www/jxcore/UnitTest_app.js, line 18 [r2] (raw file):

Mobile('ExecuteNativeTests').callNative(function (result) {

So what happens when I'm running on iOS and there is no ExecuteNativeTests?

Comments from Reviewable

Review status: all files reviewed at latest revision, 29 unresolved discussions, some commit checks failed.

src/android/test/com/test/thalitest/UTMethod, line 10 [r2] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Isn't this code actually bad in the sense that if we have included UTMethod then we did so because we expect to run tests and if tests don't run because the class isn't there then we know something horrible has gone wrong and we should have an error.

We are already logging every error, also when there is no RegisterExecuteUT file, exception is thrown so we will definitely be aware of every bad thing. If you have any suggestions  how we should do it, please write it in reply.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 659 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Actually this is a pretty seriously critical method since it what tells JXcore when we have found a peer. That's a pretty big deal. Shouldn't we test that it's getting called correctly?
>
> If you agree then please open a separate issue to add the test. I don't want to hold up this PR over writing that test.

If I understand it correctly, this method Override method  onPeerUpdated from DiscoveryManager. We are not testing DiscoveryManager but ConnectionHelper and in this class, this method only do logs. Or, you meant onPeerDiscovered and onPeerLost, this methods are actually telling jxcore about some events and I agree we should test them.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 684 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> So shouldn't we be testing that they throw? And if so then please add as a separate bug as I don't want to hold up this PR over this issue.

I agree and we are actually doing it by expecting that they will throw UnsupportedOperationException.

test/www/jxcore/UnitTest_app.js, line 18 [r2] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> So what happens when I'm running on iOS and there is no ExecuteNativeTests?

As we agreed, on iOS, method used to execute tests will have the same name. Even when there is no such registered method, which I doubt as far as this file is used specifically for running tests, log with error will occur: E/JX-Cordova: JavaCall recevied a call for unknown method ExecuteNativeTests. And it has no affect on anything at all.

Comments from Reviewable

Reviewed 5 of 5 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 15 unresolved discussions, some commit checks failed.

scripts/androidBeforeCompile.js, line 174 [r5] (raw file):

// jscs:disable jsDoc
/**
 * Updates JXcoreExtension with a method used to register the native UT executor.

NB: For some reason in the line above 'a method is' was turned into 'a method'. Please put the is back.

src/android/test/com/test/thalitest/UTMethod, line 10 [r2] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> We are already logging every error, also when there is no RegisterExecuteUT file, exception is thrown so we will definitely be aware of every bad thing. If you have any suggestions  how we should do it, please write it in reply.

I was thinking that this whole class can just go away. The native code in jxcoreextension.java should just directly call the test class. If something went wrong and the extension method isn't in jxcoreextension then we'll get an error when we call it. Better yet, we'll get an error when we compile.

src/android/test/io/jxcore/node/ConnectionHelperTest.java, line 659 [r1] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> If I understand it correctly, this method Override method  onPeerUpdated from DiscoveryManager. We are not testing DiscoveryManager but ConnectionHelper and in this class, this method only do logs. Or, you meant onPeerDiscovered and onPeerLost, this methods are actually telling jxcore about some events and I agree we should test them.

D'oh!!!! I'm sorry. I was looking at onPeerDiscovered and not onPeerUpdate. My bad.

test/www/jxcore/UnitTest_app.js, line 18 [r2] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> As we agreed, on iOS, method used to execute tests will have the same name. Even when there is no such registered method, which I doubt as far as this file is used specifically for running tests, log with error will occur: E/JX-Cordova: JavaCall recevied a call for unknown method ExecuteNativeTests. And it has no affect on anything at all.

Then that itself is a bug. :) We that exception is thrown we have to make sure that somehow or another it causes CI to be clear that this device has failed its tests. When CI gets the "No UT executed." result what will it do with it? Will it register it as a failure?

Comments from Reviewable

Review status: all files reviewed at latest revision, 14 unresolved discussions, some commit checks failed.

src/android/test/com/test/thalitest/UTMethod, line 10 [r2] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> I was thinking that this whole class can just go away. The native code in jxcoreextension.java should just directly call the test class. If something went wrong and the extension method isn't in jxcoreextension then we'll get an error when we call it. Better yet, we'll get an error when we compile.

Ok, this file will be removed, instead of we will just call the static method from RegisterExecuteUT and to do it we will need additional import. So if something will go wrong, compile error should be thrown.

test/www/jxcore/UnitTest_app.js, line 18 [r2] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Then that itself is a bug. :) We that exception is thrown we have to make sure that somehow or another it causes CI to be clear that this device has failed its tests. When CI gets the "No UT executed." result what will it do with it? Will it register it as a failure?

We will inform CI about tests results by logging specific message.

Comments from Reviewable

Test (Fail) 77622416 build is completed (6f426cd)
See https://github.com/ThaliTester/TestResults/tree/776224166f426cd_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

Test (Fail) 77622416 build is completed (6db488b)
See https://github.com/ThaliTester/TestResults/tree/776224166db488b_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

There is a problem in CI and @czyzm will investigate tomorrow

PR is added to the queue for testing as 1. task. (75fe645)

Test (Fail) 77622416 build is completed (75fe645)
See https://github.com/ThaliTester/TestResults/tree/7762241675fe645_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

PR is added to the queue for testing as 1. task. (209fb6c)

Test (Fail) 77622416 build is completed (209fb6c)
See https://github.com/ThaliTester/TestResults/tree/77622416209fb6c_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

PR is added to the queue for testing as 1. task. (2e8529c)

Test (Fail) 77622416 build is completed (2e8529c)
See https://github.com/ThaliTester/TestResults/tree/776224162e8529c_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

PR is added to the queue for testing as 1. task. (2945f57)

Test (Fail) 77622416 build is completed (2945f57)
See https://github.com/ThaliTester/TestResults/tree/776224162945f57_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

Will figure out which devices we pass on and for now we will just test on those

PR is added to the queue for testing as 1. task. (ad9274d)

Test (Success) 77622416 build is completed (ad9274d)
See https://github.com/ThaliTester/TestResults/tree/77622416ad9274d_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

Test 77622416ad9274d(ad9274d) has successfully finished without an error
See https://github.com/ThaliTester/TestResults/tree/77622416ad9274d_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

scripts/androidBeforeCompile.js, line 174 [r5] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: For some reason in the line above 'a method is' was turned into 'a method'. Please put the is back.

sorry, but I don't quite see what you mean.

Comments from Reviewable

src/android/test/io/jxcore/node/StreamCopyingThreadTest.java, line 103 [r1] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> Shouldn't we get unhappy if this is called? And actually shouldn't we test how we behave in that case?

Will be implemented as a part of the 777

Comments from Reviewable

PR is added to the queue for testing as 1. task. (b768259)

Test (Success) 77622416 build is completed (b768259)
See https://github.com/ThaliTester/TestResults/tree/77622416b768259_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

Test 77622416b768259(b768259) has successfully finished without an error
See https://github.com/ThaliTester/TestResults/tree/77622416b768259_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

I do have comments that I have included and I would ask that you resolve them but please resolve them as you see fit and then check in. This PR needs to finally be in vNext!!!! Congrats and thanks!!!!

Reviewed 6 of 8 files at r6, 1 of 3 files at r7, 1 of 1 files at r12, 1 of 1 files at r13, 1 of 1 files at r14, 1 of 1 files at r15, 3 of 3 files at r16.
Review status: all files reviewed at latest revision, 17 unresolved discussions, some commit checks failed.

build.sh, line 65 [r16] (raw file):

# Run first the tests that can be run on desktop
thali/install/setUpDesktop.sh;ERROR_ABORT
cd test/www/jxcore/;ERROR_ABORT

NB: Please put in a TODO comment here that explains why we disabled these tests.

scripts/androidBeforeCompile.js, line 6 [r16] (raw file):

var path = require('path');

// jscs:enable jsDoc

NB: Actually the jscs comments should be removed completely. E.g. we don't need //jscs:enable jsDoc, we shouldn't need anything here.

src/android/test/com/test/thalitest/RegisterExecuteUT.java, line 19 [r16] (raw file):

    }

    public static void RegisterExecuteUT() {

NB: My first reaction when I saw this code is "There are no static constructors in Java!" But of course that isn't what you are doing. But I think what you did do is a bit confusing since giving the static method the same name as the class confuses the static constructor issue. In general I think the advice given in the first answer in http://stackoverflow.com/questions/7486012/static-classes-in-java is good. Use a different name for the static method from the class. Declare the class final and make the constructor private. I would probably also throw in a 'throw' statement into the constructor just to make it clear that nobody is ever supposed to call it.

test/www/jxcore/UnitTest_app.js, line 18 [r2] (raw file):
Previously, mlesnic (Marcin Lesniczek) wrote…
> We will inform CI about tests results by logging specific message.

 Once you do, then please check in. :)

test/www/jxcore/UnitTest_app.js, line 38 [r16] (raw file):

// finish testing here (the node part will be omitted)
console.log('****TEST_LOGGER:[PROCESS_ON_EXIT_SUCCESS]****');

NB: Make me a happy person and stick a return here. The trick of telling the CI runner that we are done and having it kill the process before we get the node tests running is a little too tricky for me. If you just stick in a return then we know the node tests won't run.

thali/install/setUpTests.sh, line 41 [r16] (raw file):

cd ThaliTest
#cordova platform add ios

NB: Please stick in a comment explaining why this is commented out with a TODO on it

thali/install/setUpTests.sh, line 74 [r16] (raw file):

cordova build android --release --device

#if [ $runningInMinGw == false ]; then

NB: Please stick in a comment here as well

Comments from Reviewable

PR is added to the queue for testing as 1. task. (3c26f5f)

Test (Success) 77622416 build is completed (3c26f5f)
See https://github.com/ThaliTester/TestResults/tree/776224163c26f5f_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

Test 776224163c26f5f(3c26f5f) has successfully finished without an error
See https://github.com/ThaliTester/TestResults/tree/776224163c26f5f_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

PR is added to the queue for testing as 1. task. (c9749bc)

Test (Success) 77622416 build is completed (c9749bc)
See https://github.com/ThaliTester/TestResults/tree/77622416c9749bc_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

Test 77622416c9749bc(c9749bc) has successfully finished without an error
See https://github.com/ThaliTester/TestResults/tree/77622416c9749bc_Native_UT_for_cordova_plugin__android___555_jareksl/ for the logs

build.sh, line 65 [r16] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: Please put in a TODO comment here that explains why we disabled these tests.

Will be merged with the "Commented out ios build specific code for CI (#798)" which contains all this changes

Comments from Reviewable

Review status: 1 of 37 files reviewed at latest revision, 17 unresolved discussions, some commit checks failed.

build.sh, line 65 [r16] (raw file):
Previously, jareksl (Jarosław Słota) wrote…
> Will be merged with the "Commented out ios build specific code for CI (#798)" which contains all this changes

Done.

scripts/androidBeforeCompile.js, line 6 [r16] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: Actually the jscs comments should be removed completely. E.g. we don't need //jscs:enable jsDoc, we shouldn't need anything here.

Done.

src/android/test/com/test/thalitest/RegisterExecuteUT.java, line 19 [r16] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: My first reaction when I saw this code is "There are no static constructors in Java!" But of course that isn't what you are doing. But I think what you did do is a bit confusing since giving the static method the same name as the class confuses the static constructor issue. In general I think the advice given in the first answer in http://stackoverflow.com/questions/7486012/static-classes-in-java is good. Use a different name for the static method from the class. Declare the class final and make the constructor private. I would probably also throw in a 'throw' statement into the constructor just to make it clear that nobody is ever supposed to call it.

Done.

test/www/jxcore/UnitTest_app.js, line 18 [r2] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: Have you actually tested a failure and made sure that CI will properly fail? Once you do, then please check in. :)

yes, in case of the UT failure there is a proper comment like: Test 75789268a387c77(a387c77) has failed

test/www/jxcore/UnitTest_app.js, line 38 [r16] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: Make me a happy person and stick a return here. The trick of telling the CI runner that we are done and having it kill the process before we get the node tests running is a little too tricky for me. If you just stick in a return then we know the node tests won't run.

Done.

thali/install/setUpTests.sh, line 41 [r16] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: Please stick in a comment explaining why this is commented out with a TODO on it

Done.

thali/install/setUpTests.sh, line 74 [r16] (raw file):
Previously, yaronyg (Yaron Y Goland) wrote…
> NB: Please stick in a comment here as well

Done.

Comments from Reviewable


Scoped Library Extractor breaks build at CI (in docker container)

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior
At CI builds library extractor fails with error
><EMAIL_ADDRESS>transloco:extract-scoped-libs /home/www
> transloco-scoped-libs

Starting Transloco Scoped Libs...
✅ Copy translation from file: /libs/core/src/lib/i18n/en.json to: /apps/panel/src/assets/i18n/core/en.json
internal/fs/utils.js:220
    throw err;
    ^
Error: ENOENT: no such file or directory, open '/home/www/.gitignore'

Expected behavior
Library extractor didn't break build at CI
Minimal reproduction of the problem with instructions
Setup GitLab CI and try to build application in Docker with PROD env
What is the motivation / use case for changing the behavior?
Wee need a key to don't change gitignore or just warning/silent skip
Environment

Angular version: 8.2.14
@ngneat/transloco-scoped-libs: 1.1.0

Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

For Tooling issues:
- Node version: 12
- Platform: node:alpine in Docker

Hi @niklissad, I have fixed that and released a new version.
You are welcome to try and let us know if you run into another issue :)


Double redirect to /route-prefix/route-prefix/

What did you do?
Run prometheus with flags:
--web.external-url=https://example.com/prometheus
--web.route-prefix=/prometheus

(intended for use with a reverse proxy).  Then accessed it at /prometheus or /prometheus/
What did you expect to see?
Redirect to /prometheus/graph
What did you see instead? Under which circumstances?
Redirect to /prometheus/prometheus/graph, which gives a 404
# curl -Lv http://<IP_ADDRESS>:9090/prometheus
*   Trying <IP_ADDRESS>...
* Connected to <IP_ADDRESS> (<IP_ADDRESS>) port 9090 (#0)
> GET /prometheus HTTP/1.1
> Host: <IP_ADDRESS>:9090
> User-Agent: curl/7.47.0
> Accept: */*
>
< HTTP/1.1 301 Moved Permanently
< Location: /prometheus/
< Date: Wed, 16 Aug 2017 14:45:35 GMT
< Content-Length: 47
< Content-Type: text/html; charset=utf-8
<
* Ignoring the response-body
* Connection #0 to host <IP_ADDRESS> left intact
* Issue another request to this URL: 'http://<IP_ADDRESS>:9090/prometheus/'
* Found bundle for host <IP_ADDRESS>: 0x557ddf43ff20 [can pipeline]
* Re-using existing connection! (#0) with host <IP_ADDRESS>
* Connected to <IP_ADDRESS> (<IP_ADDRESS>) port 9090 (#0)
> GET /prometheus/ HTTP/1.1
> Host: <IP_ADDRESS>:9090
> User-Agent: curl/7.47.0
> Accept: */*
>
< HTTP/1.1 302 Found
< Location: /prometheus/prometheus/graph
< Date: Wed, 16 Aug 2017 14:45:35 GMT
< Content-Length: 51
< Content-Type: text/html; charset=utf-8
<
* Ignoring the response-body
* Connection #0 to host <IP_ADDRESS> left intact
* Issue another request to this URL: 'http://<IP_ADDRESS>:9090/prometheus/prometheus/graph'
* Found bundle for host <IP_ADDRESS>: 0x557ddf43ff20 [can pipeline]
* Re-using existing connection! (#0) with host <IP_ADDRESS>
* Connected to <IP_ADDRESS> (<IP_ADDRESS>) port 9090 (#0)
> GET /prometheus/prometheus/graph HTTP/1.1
> Host: <IP_ADDRESS>:9090
> User-Agent: curl/7.47.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Content-Type: text/plain; charset=utf-8
< X-Content-Type-Options: nosniff
< Date: Wed, 16 Aug 2017 14:45:35 GMT
< Content-Length: 19
<
404 page not found
* Connection #0 to host <IP_ADDRESS> left intact

Note: if the user goes directly to the path /prometheus/graph then they see the front page as expected.
Environment

System information:
Linux 4.4.0-87-generic x86_64

Prometheus version:
prometheus, version 2.0.0-beta.1 (branch: HEAD, revision: 4dcb465029f39a2a3030333a81753f8a40ec90ce)
  build user:       root@d3f9974fac5a
  build date:       20170811-12:12:36
  go version:       go1.8.3

Alertmanager version:
n/a

Prometheus configuration file:
default

Alertmanager configuration file:
n/a

Logs:
n/a

Note: this seems to be the same problem that #2832 was supposed to fix, which was merged on Jun 12.
However v2.0.0-beta.1 was cut on Aug 11.

@brian-brazil a #3091 has been to fix this issue. It can be closed :)

Thanks!


palemoon: 27.4.2 -> 27.5.0

27.5.1 is "out", but there is no GitHub Release
(Although, this shouldn't be an issue since it was
mostly Win10-only changes anyway)
Motivation for this change
I use this browser every day
Things done
Version bump with hash update; built successfully and am posting from the very built browser

[ ] Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)

[x] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
[ ] Tested execution of all binary files (usually in ./result/bin/)
[ ] Fits CONTRIBUTING.md.

It is almost identical to the previous version bump, 460cd807299cc6cd8497712f470318248a7bf543

cc @rnhmjoj


🍒 Cherry pick PR #15417 to staging 🍒

🍒 Cherry pick https://github.com/Expensify/App/pull/15417 to staging 🍒

🚀 Cherry-picked to staging by https://github.com/chiragsalian in version: 1.2.76-4 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.76-7 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
failure ❌

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.76-7 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
failure ❌

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.76-7 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


Revert "#7010: Replaced 'python.enableJedi' setting with 'python.languageServ…"

@erictraut File pylint.test.ts has failing tests, reverting the PR for now.

Codecov Report

Merging #7074 into master will decrease coverage by 58.3%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #7074       +/-   ##
==========================================
- Coverage    58.3%      0%   -58.31%
==========================================
  Files         483      56      -427
  Lines       21371     529    -20842
  Branches     3451      64     -3387
==========================================
- Hits        12460       0    -12460
+ Misses       8130     529     -7601
+ Partials      781       0      -781

Impacted Files
Coverage Δ

src/client/common/installer/serviceRegistry.ts
0% <0%> (-100%)
:arrow_down:

...lient/datascience/jupyter/jupyterSelfCertsError.ts
0% <0%> (-100%)
:arrow_down:

src/client/debugger/extension/serviceRegistry.ts
0% <0%> (-100%)
:arrow_down:

src/client/common/serviceRegistry.ts
0% <0%> (-100%)
:arrow_down:

src/client/interpreter/locators/services/conda.ts
0% <0%> (-100%)
:arrow_down:

src/client/common/utils/regexp.ts
0% <0%> (-100%)
:arrow_down:

src/client/common/utils/icons.ts
0% <0%> (-100%)
:arrow_down:

src/client/debugger/constants.ts
0% <0%> (-100%)
:arrow_down:

src/client/interpreter/serviceRegistry.ts
0% <0%> (-100%)
:arrow_down:

src/client/linters/serviceRegistry.ts
0% <0%> (-100%)
:arrow_down:

... and 442 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc95274...4e77600. Read the comment docs.


About stage 1 training

Thanks for your work DRM. I encountered an issue during stage 1 training in SGDET mode:
At line 61 of relation_head.py, rel_pair_idxs are extracted for the first half of proposals, and then extended to the same length as proposals using v1_idx_pairs.extend(copy.deepcopy(v1_idx_pairs)). This means that the second half of proposals has the same rel_pair_idxs as the first half, which will cause an error in code like proposal[rel_pair_idx[:, 0]] in roi_relation_feature_extractors.py. Did you have this problem in training?

hello,I'm wondering if we're experiencing the same problem. I am encountering the following error message while training for the SGDet task.This error also occurs in(dummy_y_range >= tail_proposal.bbox[:, 1].floor().view(-1, 1, 1).long()) & \of  roi_relation_feature_extractors.py.

I am looking forward to your reply.

Sorry for the late.
This version of the code is not yet able to run the SGDet, because the Faster RCNN inferred different bounding boxes for the dual view images, resulting in issues in pair matching. A feasible solution is to use Faster RCNN to infer bounding boxes in advance, and then load them during stage1 and stage2 training (this method can obtain the results in our paper). Currently, the code that can run the SGDet task has not been released yet

Thanks for your reply!


[HOLD] Web -  Wrong URL when Paypal.me payments page is closed @aneequeahmad

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Action Performed:

Go to staging.new.expensify.com
Log in with any account
Open settings page > Workspace > Manage members
Select Invite member and close the page
Open settings page > payments > Add payment method > Paypal
Close the page

Expected Result:
Url should be r/reportId
Actual Result:
Url is /settings/payments
Workaround:
Unknown
Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: <IP_ADDRESS>
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers): any
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/93399543/190194780-ef815573-4d3b-46ab-a735-adb1f8753c5f.mov
Expensify/Expensify Issue URL:
Issue reported by: @aneequeahmad
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1661466607294799
View all open jobs on GitHub

I was able to reproduce the first part where the wrong URL appeared after closing the Invite member page. I was not able to reproduce the issue with the Paypal page. As mentioned above it looks like there might be similar issues with different pages https://github.com/Expensify/App/issues/10939#issuecomment-1247831362.

I'm able to reproduce the issue, but looks like a duplicate issue. @parasharrajat could you please confirm. Thank you!

Thank you! Then I think we can close this issue and keep only one tracking issue open

Going to close this issue in favor of https://github.com/Expensify/App/issues/10939, which @parasharrajat has highlighted as having the same root cause.

Sorry, what does "add this to $11111" mean?

Ok cool, same page. One concern though is that we need to keep track of the bug reporter. It looks like we're fine for the time being, in that I can add @aneequeahmad to the master issue since now reporter is currently listed there.

Here's my summary after running through the linked reproduction steps:

We've deprecated the close button, so that's not longer a relevant part of the testing steps
Further, when I navigate through the Workspace and Payments, the URL is correct each time.

Accordingly, I think we're good to go here and we'll pay out @aneequeahmad $250 for reporting.

@JmillsExpensify agree!
Issue is not reproducible currently, but It was at that moment, so fair to pay reporting bonus

@MonilBhavsar  I agree, this is not reproducible in production but it was reproducible at that moment. So it is fixed in production

Waiting for payment here

@JmillsExpensify let's extend the contract and close this issue?

Waiting for payment here

@JmillsExpensify any update here on the payment ?

DM'ed Jason

We're working on these in bulk, as part of the react navigation project. I'll issue payment just as soon as I'm able to re-test and confirm all remaining issues are fixed. Thanks for your patience until then.

Thank you! I think it is fair to bump to this to weekly then

Still working on the testing issues.

Same

Still on hold

We're deprecating Paypal, so I'm going to issue payment for reporting and then we can close this issue.

@aneequeahmad I send you a report offer for this Upwork job: https://www.upwork.com/jobs/~01832a474eb99777e9. Let me know when you've accepted.

@JmillsExpensify i have accepted the offer. Thank you for inviting

@JmillsExpensify any update on this issue ? Lets close this one.
Also, what about other navigation issues that i reported ? Should i test them all ? If they are reproducible or not

We're closing this issue as Paypal is being deprecated. Which other issue is being referred here?

@MonilBhavsar im talking about other navigation issues that are listed in navigation project.

@JmillsExpensify i accepted the offer its been 2 weeks. Please accept it and lets close this offer. Thanks

@JmillsExpensify any update on this issue ? Lets close this one. Also, what about other navigation issues that i reported ? Should i test them all ? If they are reproducible or not

Yes please test them all and confirm if they are still reproducible or not.

@JmillsExpensify i accepted the offer its been 2 weeks. Please accept it and lets close this offer. Thanks

Paid out.


.flake8: Don't set --max-complexity if you don't care about code complexity

…plexity

(subtext: we should care about code complexity and should set --max-complexity to something vaguely reasonable)


Domain activation is Failed - [BRIEF DESCRIPTION]

App Id: d1a3ortqvlihl9
Region: ap-southeast-1
Step: Domain activation
Status: Failed

Note: Do not include information that is sensitive in nature such as your domain name, company etc.

Issue/question
A clear and concise description of what the issue/question is.
Error message
If there is an error message, please include it here.
Note: Be sure to check the message for sensitive information.
Additional information
Please add any other relevant information. Please feel free to include screenshots.

Turns out my account is not allowed to use route 53 to add DNS Zone.


Increase global_step in apply_gradients function

Short description of what this PR does:

If global_step is not None, apply_gradients increments global_step.

I understand that this is how it's typically done in optimizers, but given that the custom AdamW optimizer already doesn't do this, I don't want to change it now since people's code likely relies on the current behavior. I'll add a comment explaining that if people change the optimizer they should expect this behavior to change.


Login Application: Add PowerShell Option for Copying AWS Credential Env Vars

Describe the issue
Currently the login application has a button called "Click for Credentials" but only comes in the form of Linux based "export" commands. A team that we recently onboarded uses PowerShell and was unable to easily copy the AWS credentials. They had to manually change each export command.
Additional context

These credentials expire every hour or so, and having to change them every time would quickly become laborious
There could be other common "export" equivalents that we may want to explore

Definition of done

have an option to copy env vars in PowerShell compatible "$env:"

Can be a value add but low priority

Can be a value add but low priority


npm commands for asset-transfer-basic/chaincode-typescript fail

npm ci and npm run build for the latest asset-transfer-basic/chaincode-typescript seem to fail.
npm ci fails
npm ci fails as shown in the output below.
The reason seems to be that the latest npm-shrinkwrap.json is empty and/or out of sync with package.json.
$ cd asset-transfer-basic/chaincode-typescript
$ npm ci
npm WARN ancient lockfile
npm WARN ancient lockfile The npm-shrinkwrap.json file was created with an old version of npm,
npm WARN ancient lockfile so supplemental metadata must be fetched from the registry.
npm WARN ancient lockfile
npm WARN ancient lockfile This is a one-time fix-up, please be patient...
npm WARN ancient lockfile
npm ERR! code EUSAGE
npm ERR!
npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm ERR!
...
npm ERR! aliases: clean-install, ic, install-clean, isntall-clean
npm ERR!
npm ERR! Run "npm help ci" for more info

npm ERR!
According to this Issue, it needs to be upgraded to a higher version of TypeScript or use an older version of logform.
$ npm install
$ npm run build
><EMAIL_ADDRESS>build
> tsc

node_modules/logform/index.d.ts:14:4 - error TS1023: An index signature parameter type must be either 'string' or 'number'.

14   [key: string | symbol]: any;
      ~~~

Found 1 error.

@denyeart @bestbeforetoday

Just note that I am not a maintainer of this repository. You might want to tag @mbwhite instead.

@mbwhite

https://github.com/hyperledger/fabric-samples/pull/1018

@satota2 is this issue fixed?

The merge hasn't happened yet, but it is possible to update the TypeScript version to a version above 4.4, as demonstrated in this pull request: https://github.com/hyperledger/fabric-samples/pull/935.


Newly traced lines don't disappear when asked not to show

URL
No response
How to reproduce the issue?

Trace a line. It can be a rail or tram track, a main road, a service road, a walking path, a tree row,...
Open the Map data panel on the right and disable the features in question (railway features, main roads, service road, etc.) or disable all
The newly drawn line(s) won't disappear, even when they're not selected

Screenshot(s) or anything else?

In this case, even the nodes that the highway had in common with the tram track did not disappear, even when I deleted the tram track and asked iD to hide all features.
This might be problematic when someone needs to edit a track and a highway that have all their nodes in common (i.e. a tram track and a service road for buses on which the rails are embedded), as they won't be able to select the road to edit it.
Which deployed environments do you see the issue in?
Released version at openstreetmap.org/edit, Development version at ideditor.netlify.app
What version numbers does this issue effect?
2.25.2
Which browsers are you seeing this problem on?
Firefox

Thanks for the report. I'm able to reproduce the behavior, though I can't say whether this behavior is by design (ie, an intent to always show features created in the current session) or a bug. For now I'm tagging this as a bug.
@tyrasd - are you able to weigh in on whether this behavior is expected?

We built it this way on purpose, because otherwise you could get into the situation where a user would draw things and they'd immediately disappear, which would be more confusing..
Some history:
https://github.com/openstreetmap/iD/issues/2711#issuecomment-115822574
https://github.com/openstreetmap/iD/pull/2357

Thanks Bryan - I agree and was wondering if that was the intent - I appreciate you clarifying.
@melting-ice-cream Can you clarify the use case you have for hiding newly drawn features here? You wrote:

This might be problematic when someone needs to edit a track and a highway that have all their nodes in common (i.e. a tram track and a service road for buses on which the rails are embedded), as they won't be able to select the road to edit it.

Is the issue that you want to draw both overlapping items in the same session (not just tracing an existing line) and that you can't hide one of the new lines in order to select the other one?
In the meantime, a workaround would be to complete one of the lines, commit/upload the changes and then trace the second line in a new session, I think - then you could apply filters to the first line to work on the second. But if I'm not understanding the issue, please let me know any additional details of what you'd like to be able to do.

I've noticed there are some ways containing both tags for the road (surface, etc.) and for the tram tracks (voltage, gauge, etc.). Since the guidelines say to keep the two things separated, I wanted to:

create a new way for the tram track,
copy the railway tags from the old way to the new track,
delete the railway tags from the old way, so that it would only represent the highway(=road).
However I was unable to do step 3 because I was unable to click on the highway and it would always select the tram track.


Manually starting a CaptureStream results in a single queued buffer

I noticed a low FPS on my camera while using the crate, I think this may be the cause:
If .start() has not been called, calling the .next() method on an MMapStream queues all the buffers.
If .start() has been called it attempts to queue a single buffer.
Manually queuing the buffers at start can then cause an InvalidInput on the first call to .next() as it tries to re-queue the buffer.
Queues only a single buffer
let buffer_count = 4;
let v4l2_device = v4l::Device::with_path("/dev/video0").expect("failed to open video");
let mut stream = mmap::Stream::with_buffers(&v4l2_device, Type::VideoCapture, buffer_count)
    .expect("failed to create stream");
stream.start().expect("failed to start stream");

loop {
    let (_, meta) = stream.next().expect("failed to get image");
    println!("seq: {}, timestamp: {}", meta.sequence, meta.timestamp);
}

Fails with InvalidInput
let buffer_count = 4;
let v4l2_device = v4l::Device::with_path("/dev/video0").expect("failed to open video");
let mut stream = mmap::Stream::with_buffers(&v4l2_device, Type::VideoCapture, buffer_count)
    .expect("failed to create stream");

for index in 0..buffer_count as usize {
    stream.queue(index).expect("failed to queue buffer");
}
stream.start().expect("failed to start stream");

loop {
    let (_, meta) = stream.next().expect("failed to get image"); // <-- fails here
    println!("seq: {}, timestamp: {}", meta.sequence, meta.timestamp);
}

I believe creates expected behavior
let buffer_count = 4;
let v4l2_device = v4l::Device::with_path("/dev/video0").expect("failed to open video");
let mut stream = mmap::Stream::with_buffers(&v4l2_device, Type::VideoCapture, buffer_count)
    .expect("failed to create stream");

for index in 1..buffer_count as usize {
    stream.queue(index).expect("failed to queue buffer");
}
stream.start().expect("failed to start stream");

loop {
    let (_, meta) = stream.next().expect("failed to get image");
    println!("seq: {}, timestamp: {}", meta.sequence, meta.timestamp);
}

Thanks a lot for creating this issue. I also walked into this trap.
Your proposed fix worked for me as well.
To me the main issue seems to be that the public interface of the crate is not well structured/documented.

I second this
Was very confused on the performance difference was halving the expect fps of what my device is capable of.
for index in 1..buffer_count as usize {
    stream.queue(index).expect("failed to queue buffer");
}

was the exact fix i needed before starting the stream.
Thank you!


Feedback for “Creating a Custom Handler”

Hello! I think there is a bug in your documentation page for creating a custom cache handler. In the get method of this custom handler the implicitTags parameter is not destructured resulting in an error when the combinedTags Set is built and the get method always returns null. I've also looked at your own implementation of the redis-strings handler where the parameter is indeed destructured.
Thanks!

Hello @mihalacherazvan, thank you for pointing it out!


About training_cache and training time

Hello, I would like to ask if a large amount of time is required to create the training_cache when using navtrain for training? When I use navtrain, it usually takes around 20 hours to generate the training_cache. Are there any faster methods available? Also, I would like to know how long it took you to train Transfuser and UniAD. Thank you!

Hi @Whale-ice,
Caching the training set can take quite some time, depending on how computationally demanding your pre-processing is per sample. For example, the TransFuser baseline needs to pre-process the model input and all targets for the auxiliary tasks (e.g., the semantic maps, and bounding boxes). We still recommend caching the training set due to added stability and speed during training. The best practice is to parallelize the caching with the worker=ray_distributed argument.
TransFuser took about 24 hours on a single GPU (i.e. 100 epochs of training). UniAD took longer because a pre-training stage was involved. @sephyli might have further information. I am closing this issue for now.
All the best!
Daniel

Thank you for your reply!
Now, I have another question: i eval the ego_mlp_agent on nevtest with ckpt from https://huggingface.co/autonomousvision/navsim_baselines/tree/main/ego_status_mlp/seed 0  but get pdms 3.4*e-5

which it different from navsim benchmark
I set the train_test_split: navtest  and total 12k scenario.

oh, i find the same question as issue 28 and i settle it !


options' values' types are inconsistent

First thing is first - awesome work!
Now the issue: I'm trying to create a different template using angular-material and therefore converting all <select> elements into <md-select>
It appears that angular's select directive is more forgiving on the values, which is why code works for the default template. However, angular-materials directive is having troubles because of the lines like below and similar:
if(n && n.base && n.base === '4') {

It seems that all values are initially declared as integers. However, in the if statements the comparison uses string value. This should be:
if(n && n.base && n.base === 4) {

Hi @dmitriefimenko. This issue has popped up several times :/ the variables are changes to number, strings and back. While in the service they are strings and while in the directive they are integers if I'm remembering correctly. Either way I will take a look and find a way to make it consistent. Either only strings or only numbers. Could you post a fiddle or plunkr with some code for your issue? While the vars do change to strings and numbers and back the code accommodates for this so it shouldn't be throwing an error. Thanks!

Here is a Plunker
Notice that for the template it's using this codepen.
To see the issue, select Week in the first dropdown.
The output should be 0 0 * * 0, but it is 0 0 * * *

So turns out the root of the problem was the ng-options in the select.  I recreated the $scope.frequency to be an array of objects.  This allows the selection of an option to translate to a numeric value rather than a string so we can keep $scope.myFrequency.base as a number throughout the whole process rather than switching back and forth.  Here is your Plunkr with the updated version (1.3.2) and it is working.  Thanks for bringing this up!  You can update your bower.json to include the latest version and you should be good to go!

Confirming that it works.
For these who want to use angular-material template, here it is (no extra css needed):

    Every:
    <md-select class="cron-select" ng-model="myFrequency.base">
        <md-option ng-value="opt.value" ng-repeat="opt in frequency">{{opt.label}}</md-option>
    </md-select>
     on
    <md-select ng-show="myFrequency.base == 4" class="cron-select day-value" ng-model="myFrequency.dayValue">
        <md-option ng-value="opt" ng-repeat="opt in dayValue">{{opt | dayName}}</md-option>
    </md-select>
    = 5"> on the
    <md-select ng-show="myFrequency.base >= 5" class="cron-select day-of-month-value" ng-model="myFrequency.dayOfMonthValue">
        <md-option ng-value="opt" ng-repeat="opt in dayOfMonthValue">{{opt | numeral}}</md-option>
    </md-select>
     of
    <md-select ng-show="myFrequency.base == 6" class="cron-select month-value" ng-model="myFrequency.monthValue">
        <md-option ng-value="opt" ng-repeat="opt in monthValue">{{opt | monthName}}</md-option>
    </md-select>
    = 2"> at
    <md-select ng-show="myFrequency.base >= 3" class="cron-select hour-value" ng-model="myFrequency.hourValue">
        <md-option ng-value="opt" ng-repeat="opt in hourValue">{{opt}}</md-option>
    </md-select>
    = 3"> :
    <md-select ng-show="myFrequency.base >= 2" class="cron-select minute-value" ng-model="myFrequency.minuteValue">
        <md-option ng-value="opt" ng-repeat="opt in minuteValue">{{opt}}</md-option>
    </md-select>
     past the hour

You might even add it to docs if you'd like.
Now, this is a bit off topic, but I saw in the "Coming Soon" section about more complicated UI planned. Could you elaborate what you have in mind?

@DmitryEfimenko ability to do things like select multiple days of the week, etc.


Monk - Way of Survival - Patient Defense - No bonus unarmed attack

If I do a Flurry of Blows, I get:
Regular Attack
Regular Attack
Unarmed Attack
Flurry Attack
Flurry Attack
If I do a Patient Defense (Way of Survival gives a special version) I get:
Regular Attack
Regular Attack
This could be intended behavior but I suspect that choosing either flurry or defense should be the same 'type' of action so should result in 5 attacks vs 3 attacks.
This save is setup that you can activate one ability or the other and then attack with my monk immediately to observe.
I suspect that the patient defense is using up the bonus action when it is not supposed to.
Monk issue.zip

Flurry and Patient Defense are BA. There is nothing wrong here. Way of Survival only give ADV on attacks if you use Patient Defense.

Are you sure?
If Patient defense is using the bonus action and therefore just 2 x attacks (or whatever your base number is) is accurate, why does activating flurry still allow me to take a bonus action unarmed attack?  If they both use a bonus action I would expect that activating flurry results in:
Regular attack
Regular attack
Flurry Attack
Flurry Attack
Since the unarmed attack is a bonus action.

You probably have a feat allowing that.

Oh, I do have Pugilist. Even still, if Pugilist grants a bonus attack in one scenario, shouldn't it do the same in the other?

That does make sense.  It is still worth picking up for a monk for the extra d4 damage and occasional ki-less attacks.  Here is a screenshot of my monk settings within the mod if it helps.


Changed order between customizing via enricher and enriching labels.

We need to extend the abstraction to allow a preEnriching() to build up default objects and a postEnriching() to customize after the label / annotation enricher has been called. Need to think about it, in order to make it not too complicated.
Fixes #72

[merge]


1.4.3 Gantt Chart

Screenshot of the gantt chart which was worked by WeiQi together with Jansen.


Remove trivial nodes before building subdag

Resolves #7195
Description
This implements the "smart trimming" suggested in (https://github.com/dbt-labs/dbt-core/issues/7195).
Local testing on my project:
DBT 1.4.4:
dbt build --select modelname
00:31:26  Running with dbt=1.4.4
00:31:27  Found 307 models, 3708 tests...
00:34:02
00:34:08
00:34:09  Concurrency: 16 threads (target='default')
00:34:09
00:34:09  1 of 179 START sql table model  .................................... [RUN]

Build time: 2 minutes 45 seconds
Version 1.5.0-b4:
dbt build --select modelname
00:39:02  Running with dbt=1.5.0-b4
00:39:05  Found 307 models, 3708 tests...
00:41:14
00:41:19
00:41:20  Concurrency: 16 threads (target='default')
00:41:20
00:41:20  1 of 179 START sql table model  .................................... [RUN]

Build time: 2 minutes 18 seconds
This branch:
dbt build --select modelname
05:28:42  Running with dbt=1.5.0-b4
05:28:45  Found 307 models, 3708 tests...
05:28:45
05:28:52
05:28:52  Concurrency: 16 threads (target='default')
05:28:52
05:28:52  1 of 179 START sql table model  .................................... [RUN]

Build time: 10 seconds
Checklist

[X] I have read the contributing guide and understand what's expected of me
[X] I have signed the CLA
[X] I have run this code in development and it appears to resolve the stated issue
[X] This PR includes tests, or tests are not required/relevant for this PR
[X] I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
[X] I have run changie new to create a changelog entry

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.
In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.
CLA has not been signed by users: @ttusing

Believe this is in error! Filled it out a few minutes before opening PR, might be a timing issue.

Overall this looks like an exciting improvement. We're thinking through whether it is likely to work well for all of the graph topologies that come up in practice, but I'm pretty optimistic about that.

Overall this looks like an exciting improvement. We're thinking through whether it is likely to work well for all of the graph topologies that come up in practice, but I'm pretty optimistic about that.

I think this is strictly better than random selection in all cases, especially with switching to the product of in_degree and out_degree.

@iknox-fa Looks like the issues we discussed have been addressed here. What do you think about this PR?

@iknox-fa Looks like the issues we discussed have been addressed here. What do you think about this PR?

@iknox-fa  If part of the hesitation is the pruning part and the sorting is good to go, I can open a separate PR.

@iknox-fa @jtcohen6 (cc: @leahwicz)
I'd like to approve this PR and get it into the RC for 1.5, and what follows is my case for doing that.
What we know:

The code is correct. There is no disagreement on that point.
It is 5-10x faster on long runs involving narrow node selections on large graphs, as benchmarked by @ttusing and at least one other user.
The code involves heuristic changes to the order in which nodes are removed from the graph, and there is a strong basis for concluding that the heuristic will improve performance across a variety of graph types.

Based on my general algorithmic/performance experience, I'd guess the following:

There is a 85% chance that this is faster for all users all of the time.
There is a 10% chance that this is faster for users with graphs similar to the ones the change has been tried on, but no change or slightly slower for other graphs.
There is 5% chance that the algorithm is significantly slower for some users, even though it dramatically improves performance in other cases.

Since we will have time to test the performance in the wild during the RC interval, I think it is worth a 5% risk of some performance regressions if it lets us dramatically improve the experience of many users in the 95% case. Even if there are performance regressions, I believe we could probably address them by rolling forward instead of back, but rolling back is easy.

@peterallenwebb Thanks for laying out the rationale - I'm down to give this a shot, and merge for inclusion in v1.5.0-rc1.

@peterallenwebb thanks as well for the analysis. I feel good with getting this in the RC based off of that and monitoring to see if we see any regressions in perf from different DAG shapes.


How to extract just metabolic subset of genes?

I am interested in downloading metabolic enzymes from pathways. For example in the omega3 senescence pathway (https://www.wikipathways.org/pathways/WP5424.html) there are various genes that are not directly linked to metabolism, including p21. I think it it should be possible to identify metabolism genes using all genes involved in conversion MIM interactions? Is there a method of just extracting these genes as opposed to all genes in the pathway using the R package?
Thanks

@DeniseSl22, didn't we write a SPARQL query for this at some point in time? Or was that just on my long wish-/todo list?

The pathway WP5424 is not in the RDF yet, but the following SPARQL should give you some idea how to do this:
SELECT ?wpid ?catalyst ?source ?target WHERE {
  ?pathway a wp:Pathway ;
      dc:identifier / dcterms:identifier ?wpid .
  ?catalysis a wp:Catalysis ;
    dcterms:isPartOf ?pathway ;
    wp:source / rdfs:label ?catalyst ;
    wp:participants ?reaction .
  ?reaction a wp:Interaction .
  OPTIONAL { ?reaction wp:source ?source }
  OPTIONAL { ?reaction wp:target ?target }
} ORDER BY ASC(?catalysis)


The v8 of YodaRT

Now we have the following UIs to work with:

Shell UI is to represent the user interface includes light and audio effects.
Voice UI is to handle all voice-based user interface requests.
Display UI is to handle the requests to render for products with screen(s).

The basic services are:

MediaPlayerDaemon is a service that controls all the media tasks.
TextToSpeechDaemon is to control all the TTS requests.
BluetoothService is the service that manages all the bluetooth tasks and states, also the abstract layer for different low-level implementations like: bsa_server or bluez.
Activation is to optimize the awaken user experience, but only controls the sound.
PulseAudio is still awesome software that we should use more to manage the sounds.

The above are just naming optimization than our current implementation, not big addresses and improvements.
Redefinition of YodaRT
We have to refine the meaning of YodaOS Runtime, namely YodaRT. In our implementation, this runtime includes all basic services implementations and libraries, which might be not clear for any incoming developers.
Thus, as the first step of starting the next parts, I propose that this runtime includes the following:

The base libraries which abstracts the functions that the framework layer would use, we call it “yoda library”.
The core UI components source code, that includes: shell, voice and display.

So the new root directory would only contains: lib, shell, voice and display.
Yoda Flow
Except for the shell, voice and display components, we should have a component named as “flow”, which is the tiny core to provide the minimum support for all the use cases.
The Flow should own the following abilities that we resolved in v7:

Apps loader.
Apps scheduler.
Custodian for network manager.
Dbus expose.
Keyboard controls.
Lifetime of applications and skills.
Permission controls.
MQTT wormhole.

To be continue, but any ideas is also welcome to chime in :)

/cc @Rokid/nodejs

Closing this because the YodaOS 8 refactoring has been done!


break out a new doc section about performance

This change helps keep the section on configuration shorter and sweeter, and lets me address topics in performance more directly rather than as asides.

Merged https://github.com/hibernate/hibernate-reactive/pull/350
Thanks


research: plugin creation with editor js

we use editor js for the generator. new widgets are plugins in editor js.

A plugin is a class that has to contain a render() and a save(blockContent) method.
In the render() function you specify the different elements of the widget and return the element containing them.
The save(blockContent) method gets the current contend of the widget and should return a json containing the data of the element for the editor js page. More can be found at the official docu page at https://editorjs.io/


set_container_access_policy returns 400 Bad Request on a valid request

Which service(blob, file, queue, table) does this issue concern?
blob
Which version of the Azurite was used?
3.29.0
Where do you get Azurite? (npm, DockerHub, NuGet, Visual Studio Code Extension)
DockerHub
What's the Node.js version?
--
What problem was encountered?
API set_container_access_policy returns with 400 Bad Request on a valid request. The same request succeeds with Azure (Requests were made using the Python SDK).
Steps to reproduce the issue?
The following code should reproduce it
from azure.storage.blob import BlobServiceClient
from azure.storage.blob import PublicAccess
from azure.storage.blob import AccessPolicy

blob_service_client = BlobServiceClient.from_connection_string(f'{conn_string}')
container_client = blob_service_client.get_container_client('container')

container_client.create_container()
container_client.set_container_access_policy(signed_identifiers={'test': AccessPolicy()}, public_access=PublicAccess.Container)

azure.core.exceptions.HttpResponseError: Bad Request
ErrorCode:None

If possible, please provide the debug log using the -d parameter, replacing <pathtodebuglog> with an appropriate path for your OS, or review the instructions for docker containers:
-d "<pathtodebuglog>"

Please be sure to remove any PII or sensitive information before sharing!
The debug log will log raw request headers and bodies, so that we can replay these against Azurite using REST and create tests to validate resolution.
Have you found a mitigation/solution?

Hi @anna-mazhar ,  If possible, can you share Azurite debug log?

Sure, here is the log generated with the code above: debug.log

@anna-mazhar
From the debug log, we see the error is "containerAcl.AccessPolicy cannot be null or undefined."
Could you share the user scenario why you are trying to put a null/undefined access policy the container?


Fix typescript import

Resolves issue #129
The easiest and quickest solution I can see here is to just effectively use a named export for both js and ts.
This results in much tidier imports for typescript projects, too.

Closing due to lack of follow up.


Configuration not persisted

Every time that I restart WebStorm, my task server dissapears. It does not persist the configuration apparently.
Steps to reproduce:

Open Webstorm
Add a task server
Close Webstorm
Open webstorm and validate that the task server is no longer available on the list

Thanks for raising the issue. I've fixed it in Configuration not persisted. Will release a new version in a couple of days. Feel free to reopen this if the issue still persists.


Use file URI to represent paths

This is fairly large that changes the internal representation of paths to use file:// URIs. This fixes a few bugs and means that relative paths are now relative URIs.

Codecov Report

Merging #149 into master will increase coverage by 1.1%.
The diff coverage is 37.83%.

@@           Coverage Diff            @@
##           master    #149     +/-   ##
========================================
+ Coverage    40.5%   41.6%   +1.1%
========================================
  Files          36      36
  Lines         958     971     +13
========================================
+ Hits          388     404     +16
+ Misses        570     567      -3

Impacted Files
Coverage Δ

src/lib/target-tasks/mocha.ts
54.54% <ø> (ø)
:arrow_up:

src/lib/target-tasks/mocha-main.ts
57.14% <ø> (ø)
:arrow_up:

src/lib/project-tasks/tsconfig-json.ts
0% <0%> (ø)
:arrow_up:

src/lib/target-tasks/tsconfig-json.ts
53.33% <0%> (ø)
:arrow_up:

src/lib/utils/branch-publish.ts
0% <0%> (ø)
:arrow_up:

src/lib/project-tasks/bump-minor.ts
20% <0%> (ø)
:arrow_up:

src/lib/utils/project.ts
0% <0%> (ø)
:arrow_up:

src/lib/project-tasks/bump-patch.ts
20% <0%> (ø)
:arrow_up:

src/lib/utils/git.ts
0% <0%> (ø)
:arrow_up:

src/lib/targets/mocha.ts
76.81% <0%> (ø)
:arrow_up:

... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f63aca...7534969. Read the comment docs.


ID late fees:
User feedback: language
(late_fees_assessed)

Question ID
late fees

Details
"Did your landlord say that you owe fee for paying rent late?" Should say "owe a fee" or "owe fees"

Variable being sought
late_fees_assessed

Package version
1.0

Maturity target
Level 4 - this is purely an enhancement to improve user experience

Filename
docassemble.MOHUDEvictionProject:data/questions/MOHUDEvictionProject.yml

Did your landlord say that you owe fees for paying rent late?: late_fees_assessed
datatype: yesnoradio


**kwargs supplied to css() or as Tag attributes should not have their key's transformed

css()
Currently, css() upgrades all instances of _ to -.
This makes it impossible to create a css variable of --_foo. css(**{"--_foo": "bar"}) will get transformed to ---foo:bar;.
Tag Attributes
Similarly, you can not create an attribute that contains _. Ex: div({"data-foo_bar": "baz"}) returns

I do not know of a clean solution for the TagAttrDict to not upgrade some keys while continue to upgrade others as all keys are strings.
Taking the extreme position... I don't believe TagAttrDict should upgrade any attribute keys. If a user wants to provide difficult to define attrs, they can use div({"impossible-attr": "value"}) or div(**{"impossible-attr": "value"}).  Unlike R's list(), there isn't a convenient/pretty way in python to supply keys to a dictionary.
A possible work around would be to supply a class for attributes that should be treated "as is".  Ex: div(TagAttrsRaw(**{"_impossible-attr_": "value"})) which would give

Possible solution for css() is to add a new method of css_raw() which will not update the keys and only concatenate the values. I believe we could drop collapse_= arg as I haven't used anything other than "".
Almost like a css(upgrade_keys=False)... However, I do not want to add this argument to the css() function.

For reference, this is how we handled the same (or similar) problem in htmltools: https://github.com/rstudio/htmltools/pull/402

I like R htmltools approach. Hate to throw the baby out with the bathwater here, when the common case surely is things like css(background_color="#F0F0F0").
@schloerke are there HTML attributes that have _?

I'd be ok with not upgrading keys that start with --. (R htmltools approach)
I don't know of any legitimate HTML attrs that have a _.

I guess I was in a "R is magical" and "python should not perform magic" mood.  With python having a way to supply arguments that would not need to be upgraded, I wanted to keep that mindset as much as possible. And adding the R htmltools change seemed like it was adding more confusion as to what is being upgraded when.
I'm ok if we just ignore CSS variables (or keys that start with --).

I'm ok if we just ignore CSS variables (or keys that start with --).

That's what makes sense to me. I think we might have different definitions of "magic" in any case... to me this is just convenience, whereas "magic" means the user has no reasonable way to form a mental model for what's happening (like if you introduced NSE to Python, THAT would be magic).

Thank you both! I believe we have a clear path forward.
(... Use non-standard evaluation within python. 😜 )


Fix rubocop redundant freeze warnings

Fix Rubocop freeze warnings.
Rubocop is warning about things that are double frozen, ie .freeze is redundant.
These warnings started showing up as part of the Ruby 3.1 upgrade, and
were excluded from that PR in order to minimize noise.

Note to self before merge: probably also want to enable the Style/FrozenStringLiteralComment cop for consistency across the codebase... ie, that we don't have some files using the magic comment and other files where we explicitly freeze everything.

I had to do a painful rebase on this one, and easily could have made a mistake. So once #5447 is merged and this is rebased on top of that, do a careful inspection of the changes to ensure I didn't screw it up.

Merging once passes CI as this is all very safe lint fixes


Retry timeout errors in the bag verifier

I don't think I understand your comment about the S3FixityChecker? I don't think I put the retry logic there unless I did something weird yesterday evening. The main changes in the FixityCheckers were meant to be around dependency injection cause I forgot them in the other PR but I was trying to write a test that was as close as possible to the real case, so with a failing streamReader used in the fixityChecker

Sorry, not the fixity checker, the S3 bag verifier. It was looking at this line:
override def isRetriable(error: BagVerifierError): Boolean = false

which made me wonder “hmm, why doesn’t the S3 bag verifier do any retrying 🤔” that made me realise it’s because we catch retryable S3 errors elsewhere.

ah ok makes sense. I had found where the error gets thrown but I had missed how the retry logic works. I'll change to use AzureStorageErrors


Spanner Batch client

Add Spanner Batch client.
Batch client is useful when one wants to read or query a large amount of data from Cloud Spanner across multiple processes. It allows to partition specified read/query and then read data over each partition independently yet at the same snapshot.

All authors are ok with their commits being contributed.

The commits attributed to me are mine and I want them part of this PR.


Drawer Nav Item using Link as the component

Fixes #182

Changes proposed in this Pull Request:
I am not 100% sure if this change to the drawer component is necessary. I just realized that you are able to do { to: '/overview', component: Link } to InfoListItemProps and get it to work. Doing what I am doing here might make it a breaking change.

Make drawer render a <Link> component from react-router-dom when a to attribute in the InfoListItemProps is present
react-router-dom is now listed as a peer dependency
Update the ILI & drawer docs to offer suggestions on how to render links

Screenshots:
To verify my work, if I do this to line 107 of ./demos/showcase/src/router/NavigationDrawer.tsx, I will get this screenshot
                        {
                            title: titleList[0],
                            itemID: titleList[0],
                            icon: <ListIcon className={clsx({ [classes.iconFlip]: rtl })} />,
                            onClick: (): void => {
                                // navigate(0);
                                if (xsDown) dispatch({ type: CLOSE_DRAWER });
                            },
                            // @ts-ignore
                            InfoListItemProps: { to: '/overview' },
                        },

it will be able to update the current location, can be opened in a new tab with the correct address, the tooltip thing shows up in the bottom left corner of the browser, and upon clicking the browser does not refresh (which suggest that we are using <Link> here). Note that the background shading is controlled by a different mechanism and is not updated.
As a comparison, other drawer nav items stay the same way:

I am adding the component prop to InfoListItem instead because that's where the problem stems...
Although it turns out you will still have to ts-ignore something, because to is not part of the InfoListItemProps.

Update:
From our discussion, we came to realize that MUI is changing their ListItemProps dynamically with some generic type tricks. Here are two screenshots from our experiment:

We agreed to close this PR for now. I will add a little more explanation in the enhancement.

@joebochill should be good to go


其他类型的题目

怎么能显示其他类型的题目呢，比如database 之类的

currently we don't support that

feel free to try release 1.0.0, database/shell problems are supported.


Error on const, shader validation on older shader language version ?

Hi,
first of all, this extension is great. GG
I tried to use in vscode but it seems that the validation produces errors when chrome canary is fine with the shader. It seems like the validation is not up to date with the shading language.
on this one the first line produces an error.
--> webgpu-experiment/src/node-functions.wgsl:2:1
error: error: expected global item ('struct', 'let', 'var', 'type', ';', 'fn') or the end of the file, found 'const'
┌─ wgsl:2:1
│
2 │ const NodeMaxLevel = 14;
│ ^^^^^ expected global item ('struct', 'let', 'var', 'type', ';', 'fn') or the end of the file
Note that this shader is injected a bit like include in a more complex compute shader and vertex shader. Still It feels that it validates on a different version and the language.
node-functions.txt

const are not yet supported in naga: https://github.com/gfx-rs/naga/issues/2071 should be fixed in https://github.com/gfx-rs/naga/pull/2075
That being said, naga was indeed outdated, so I updated it, and published cargo-wgsl 0.0.12

🙏 thx

I'd like to keep that open, if you don't mind. Just to keep track of that.

Ho np 👍

New versions are released with const support.

New versions are released with const support.

Great will take a look at that, thanks.

Indeed it is now working as expected. cargo-wgsl is also updated already so vscode-wgsl just works :tm:


on "Select" step in pipeline setup, should "All" be the default

Migrated from GitLab: https://gitlab.com/meltano/meltano/-/issues/907
Originally created by @dmor on 2019-08-21 22:40:33

Seems like in every case except for Salesforce this is totally fine, and Saleforce has a performance risk. Could we limit the orange scary message to just that tap? Could we also make "All" the default setting? It would reduce one click in the signup flow, per my speed run :)

View 3 previous comments from the original issue on GitLab


Add validations in fromJsonFile in LogAgentConfig file.

Relevant code path - https://github.com/sagar-arora/LogAgent/blob/2739844c3809e2ce928c8988f4a4977456906d23/src/main/java/com/arorasagar/logagent/LogAgentConfig.java#L88
Add validations for LogAgentConfig file for required fields -
String tmpDir;
  int delayBetweenCycles;
  String recoveryPath;
  int retentionPeriod;
  String endPointType;

delayBetweenCycles is required and should be > 0.
recoveryPath is optional.
retentionPeriod is required and should be > 0.
endPointType is required and should be valid string. Additionally we can check if it's s3 path, gcs path or azure files path.

Hi Sagar, I would like to contribute with this issue. Could you assign this to me?

Hi Melvin, assigned you this issue. I will create more issues/stories for the project. Let me know if you have any questions or  you face any difficulty setting up etc.

Hi Sagar, Can you assign me this issue..I would like to work on it...

I'm sorry Sagar, I couldn't work in this issue because I changed my work and I'm still in training. I left free for @akashshirale14

Hey Akash, assigned the issued to you.

Okay, Thank you. I will finish these 2 and then work on more...👍

Hi Sagar,
I was trying to work on this issue using Hibernate validator.
But while trying to run the project on my local setup I am getting this error message
2022-10-03 19:03:07,896 DEBUG [main] (CsmConfigurationProviderChain.java:48) - Unable to load configuration from com.amazonaws.monitoring.EnvironmentVariableCsmConfigurationProvider@18518ccf: Unable to load Client Side Monitoring configurations from environment variables! 2022-10-03 19:03:07,897 DEBUG [main] (CsmConfigurationProviderChain.java:48) - Unable to load configuration from com.amazonaws.monitoring.SystemPropertyCsmConfigurationProvider@1991f767: Unable to load Client Side Monitoring configurations from system properties variables! 2022-10-03 19:03:07,898 DEBUG [main] (Cs [Config.json.txt](https://github.com/sagar-arora/LogAgent/files/9698110/Config.json.txt) mConfigurationProviderChain.java:48) - Unable to load configuration from com.amazonaws.monitoring.ProfileCsmConfigurationProvider@659eef7: Unable to load config file
I have followed the instructions in Readme.md and updated the Config File(attached).
Also created an S3 bucket in AWS, setup an IAM user, assigned policy to allow access to the S3 bucket and created a AWS credentials file at "%USERPROFILE%.aws\credentials" on my local.
Can you help me with setting up the project ?
Note : I am a complete beginner with AWS and might have missed something pretty basic.
Regards,
Krishna Gupta

Hi @sagar-arora - I'd like to contribute to this issue..

Hi @SoorajSundar1505 assigned it to you.

@sagar-arora if no-one is working on this issue still , could you please assign it to me? I am interested in contributing

@sagar-arora I would like to contribute
could you please assign the issue to me?

@sagar-arora I have solved the issue
Kindly review it , a humble request to add hactober tag as well :)


Skype web sdk sample issue.

When i am running skype websdk i am getting below error.

I am facing the same issue. Did you resolve it by any chance?

@parmodmalik @KiwiHub I was facing the same issue also
Solution:
Run the 3 power shell scripts in this folder: skype-docs\Skype\WebSDK\build

It will compile the utils/video-utils.ts and generate the config.js file


OpsGenie Integration - Additional fields

Hello,
At the moment, when an alert is sent from Kapacitor to the OpsGenie API only the message and Tags are included in the alert payload, meaning we need to use string processing to get basic information out of an alert message. Things like percent free space and free GB included as fields in the payload would be awesome. Then users can access the data in the alerts using OpsGenies .extraProperties function. Then we can fire alerts to OpsGenie with all the information included and parse out the data we need.
I've tried to include .details in the alert payload and parse the info from there but this hasn't worked.
Currently tags can be accessed in OpsGenie with _payload.extraProperties.tagname
Something similar with fields would be preferable. Is this possible, if not can it be made possible?
I've asked at OpsGenie about this, they say the integration isn't maintained by them so the best they can do is open a feature request. Thought i'd ask too.
Let me know if any other info is required :)

@bogski87 thanks for the issue. We will take a look and consider it for a future release of Kapacitor.


can i use the ionic json file in my vscode plugin?

I don't know whom to ask this, so i raise a ticket here,
currently i am prepare a plugin for vscode,
shall i use the data.json in my plugin

Not relevant